[PATCH v4 0/3] platform: arm64: Huawei Matebook E Go embedded controller

[PATCH v4 0/3] platform: arm64: Huawei Matebook E Go embedded controller

[Pengyu Luo]

This adds binding, drivers and the DT support for the Huawei Matebook E Go
(sc8280xp-based) Embedded Controller which is also found in Huawei Matebook
E Go LTE (sc8180x-based), but I don't have the sc8180x one to perform
tests, so this series enable support for sc8280xp variant only, this series
provides the following features:

- battery and charger information report
- charging thresholds control
- FN lock (An alternative method)
- LID switch detection
- Temperature sensors
- USB Type-C altmode
- USB Type-C PD(high power)

Thanks to the work of Bjorn and Dmitry([1]), the work of Nikita([2]),
writing a EC driver won't be suffering. This work refers a lot to their
work, also, many other works. I mentioned them in commit messages.

Depends: https://lore.kernel.org/linux-arm-msm/20241220160530.444864-1-mitltlatltl@gmail.com

[1] https://lore.kernel.org/all/20240614-yoga-ec-driver-v7-0-9f0b9b40ae76@linaro.org/
[2] https://lore.kernel.org/all/20240315-aspire1-ec-v5-0-f93381deff39@trvn.ru/

base-commit: b323d8e7bc03d27dec646bfdccb7d1a92411f189

Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
---
Changes in v4:
- use new API to register hwmon device instead of the deprecated one. (Guenter)
- add Reviewed-by tag for dt-binding (Krzysztof)
- drop unnecessary header (Ilpo)
- use guard mutex (Ilpo)
- improve comments and naming (Ilpo)
- add a shallow copy version of extr_resp() (Ilpo)
- add functions to handle resp and req whose size is 1
- drop PSY and UCSI subdrivers, commit them once the base driver is upstreamed
- Link to v3: https://lore.kernel.org/linux-arm-msm/20250113175049.590511-1-mitltlatltl@gmail.com

Changes in v3:
- Link to v2: https://lore.kernel.org/linux-arm-msm/20250105174159.227831-1-mitltlatltl@gmail.com

dt-binding:
- drop generic compatibles. (Krzysztof)
- remove '+' to use literal block style. (Krzysztof)

ec:
- take struct gaokun_ucsi_reg as parameter (Heikki)
- add almost all kernel doc comments (Krzysztof, Heikki)

ucsi:
- drop unnecessary ucsi quirks (Dmitry)
- add UCSI v1.0 to ucsi.h (Heikki)
- use gaokun_ucsi_read_cci() to read cci directly (Heikki)
- drop unnecessary gaokun_ucsi_get_port_num (Heikki)
- rename member port_num => num_ports (Heikki)
- fix completion, forgot to signal threads in previous version

dt:
- fix indentation (Konrad)
- add a link between role switch and connector

Changes in v2:
- Link to v1: https://lore.kernel.org/linux-arm-msm/20241227171353.404432-1-mitltlatltl@gmail.com

global:
- drop qcom's products(i.e. sc8180x, sx8280xp) everywhere, use 'product'-based instead(Krzysztof, Bryan)
- drop Cc Nikita Travkin, we had discussed the device in PM.
- add myself to MAINTAINERS

dt-binding:
- fix building (Rob Herring (Arm))
- remove unnecessary code (Krzysztof)
- add bugzilla documentation, insights of gaokun(see [1] or patch[1/5]) (Krzysztof, Aiqun(Maria))
- explain the difference between PMIC GLink and gaokun EC (Aiqun(Maria))

ec:
- use Linux style comments (Krzysztof)
- add a comment for mutex lock (Krzysztof)
- add more kerneldoc for exported functions (Krzysztof)
- eliminate unnecessary conditions (Bryan)
- add a macro for check thresholds (Bryan)
- improve English (Bryan)
- use existing sysfs interface(hwmon, psy) whenever possible (Krzysztof)
- use __le16 and related endianess conversion function for temp data (Ilpo)
- drop alias for packet headers (Ilpo)
- avoid hardcoding i2c msgs size (Aiqun(Maria))
- add a comment for the sleep in critial region (Bryan, Aiqun(Maria))
- use macro to construct packet (Bryan, Aiqun(Maria))

wmi:
- dropped

ucsi:
- reorder headers (Bryan)
- a comment for the orientation map macro (Bryan)
- make mux mode map more explicit(minus six is very clear now) (Bryan, Dmitry)
- handle port update exceptions return (Bryan)
- a comment for the UCSI quirks (Dmitry)
- use the inline hint for the short register function (Dmitry)
- use the API with delay to handle register instead of a direct sleep (Bryan)
- handle unfinished initialization early

psy:
- add charging related sysfs to here (Krzysztof, Dmitry)
- document ABI for power_supply sysfs (Krzysztof)
- drop charging threshold, use smart charging instead

dts:
- correct indentation, properties' order. (Konrad)

Pengyu Luo (3):
  dt-bindings: platform: Add Huawei Matebook E Go EC
  platform: arm64: add Huawei Matebook E Go EC driver
  arm64: dts: qcom: gaokun3: Add Embedded Controller node

 .../bindings/platform/huawei,gaokun-ec.yaml   | 124 +++
 MAINTAINERS                                   |   7 +
 .../boot/dts/qcom/sc8280xp-huawei-gaokun3.dts | 163 ++++
 drivers/platform/arm64/Kconfig                |  21 +
 drivers/platform/arm64/Makefile               |   1 +
 drivers/platform/arm64/huawei-gaokun-ec.c     | 787 ++++++++++++++++++
 .../linux/platform_data/huawei-gaokun-ec.h    |  80 ++
 7 files changed, 1183 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/platform/huawei,gaokun-ec.yaml
 create mode 100644 drivers/platform/arm64/huawei-gaokun-ec.c
 create mode 100644 include/linux/platform_data/huawei-gaokun-ec.h

-- 
2.48.1

[PATCH v4 1/3] dt-bindings: platform: Add Huawei Matebook E Go EC

[Pengyu Luo]

Add binding for the EC found in the Huawei Matebook E Go and
Huawei Matebook E Go LTE 2-in-1 tablets, the former one is a QS sc8280xp
based tablet, the latter one is a QS sc8180x based tablet.

This series has a codename, gaokun. More information about gaokun, please
check https://bugzilla.kernel.org/show_bug.cgi?id=219645

Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/platform/huawei,gaokun-ec.yaml   | 124 ++++++++++++++++++
 1 file changed, 124 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/platform/huawei,gaokun-ec.yaml

diff --git a/Documentation/devicetree/bindings/platform/huawei,gaokun-ec.yaml b/Documentation/devicetree/bindings/platform/huawei,gaokun-ec.yaml
new file mode 100644
index 000000000..4a03b0ee3
--- /dev/null
+++ b/Documentation/devicetree/bindings/platform/huawei,gaokun-ec.yaml
@@ -0,0 +1,124 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/platform/huawei,gaokun-ec.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Huawei Matebook E Go Embedded Controller
+
+maintainers:
+  - Pengyu Luo <mitltlatltl@gmail.com>
+
+description:
+  Different from other Qualcomm Snapdragon sc8180x and sc8280xp-based
+  machines, the Huawei Matebook E Go tablets use embedded controllers
+  while others use a system called PMIC GLink which handles battery,
+  UCSI, USB Type-C DP Alt Mode. In addition, Huawei's implementation
+  also handles additional features, such as charging thresholds, FN
+  lock, smart charging, tablet lid status, thermal sensors, and more.
+
+properties:
+  compatible:
+    enum:
+      - huawei,gaokun3-ec
+
+  reg:
+    const: 0x38
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  interrupts:
+    maxItems: 1
+
+patternProperties:
+  '^connector@[01]$':
+    $ref: /schemas/connector/usb-connector.yaml#
+
+    properties:
+      reg:
+        maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        embedded-controller@38 {
+            compatible = "huawei,gaokun3-ec";
+            reg = <0x38>;
+
+            interrupts-extended = <&tlmm 107 IRQ_TYPE_LEVEL_LOW>;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            connector@0 {
+                compatible = "usb-c-connector";
+                reg = <0>;
+                power-role = "dual";
+                data-role = "dual";
+
+                ports {
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+
+                    port@0 {
+                        reg = <0>;
+
+                        ucsi0_ss_in: endpoint {
+                            remote-endpoint = <&usb_0_qmpphy_out>;
+                        };
+                    };
+
+                    port@1 {
+                        reg = <1>;
+
+                        ucsi0_sbu: endpoint {
+                            remote-endpoint = <&usb0_sbu_mux>;
+                        };
+                    };
+                };
+            };
+
+            connector@1 {
+                compatible = "usb-c-connector";
+                reg = <1>;
+                power-role = "dual";
+                data-role = "dual";
+
+                ports {
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+
+                    port@0 {
+                        reg = <0>;
+
+                        ucsi1_ss_in: endpoint {
+                            remote-endpoint = <&usb_1_qmpphy_out>;
+                        };
+                    };
+
+                    port@1 {
+                        reg = <1>;
+
+                        ucsi1_sbu: endpoint {
+                            remote-endpoint = <&usb1_sbu_mux>;
+                        };
+                    };
+                };
+            };
+        };
+    };
-- 
2.48.1

[PATCH v4 2/3] platform: arm64: add Huawei Matebook E Go EC driver

[Pengyu Luo]

There are three variants of which Huawei released the first two
simultaneously.

Huawei Matebook E Go LTE(sc8180x), codename seems to be gaokun2.
Huawei Matebook E Go(sc8280xp@3.0GHz), codename must be gaokun3. (see [1])
Huawei Matebook E Go 2023(sc8280xp@2.69GHz), codename should be also gaokun3.

Adding support for the latter two variants for now, this driver should
also work for the sc8180x variant according to acpi table files, but I
don't have the device to test yet.

Different from other Qualcomm Snapdragon sc8280xp based machines, the
Huawei Matebook E Go uses an embedded controller while others use
a system called PMIC GLink. This embedded controller can be used to
perform a set of various functions, including, but not limited to:

- Battery and charger monitoring;
- Charge control and smart charge;
- Fn_lock settings;
- Tablet lid status;
- Temperature sensors;
- USB Type-C notifications (ports orientation,  DP alt mode HPD);
- USB Type-C PD (according to observation, up to 48w).

Add a driver for the EC which creates devices for UCSI and power supply
devices.

This driver is inspired by the following drivers:
        drivers/platform/arm64/acer-aspire1-ec.c
        drivers/platform/arm64/lenovo-yoga-c630.c
        drivers/platform/x86/huawei-wmi.c

[1] https://bugzilla.kernel.org/show_bug.cgi?id=219645

Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
---
 MAINTAINERS                                   |   7 +
 drivers/platform/arm64/Kconfig                |  21 +
 drivers/platform/arm64/Makefile               |   1 +
 drivers/platform/arm64/huawei-gaokun-ec.c     | 787 ++++++++++++++++++
 .../linux/platform_data/huawei-gaokun-ec.h    |  80 ++
 5 files changed, 896 insertions(+)
 create mode 100644 drivers/platform/arm64/huawei-gaokun-ec.c
 create mode 100644 include/linux/platform_data/huawei-gaokun-ec.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 33b9cd11a..27ff24e7d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10692,6 +10692,13 @@ S:	Maintained
 F:	Documentation/networking/device_drivers/ethernet/huawei/hinic.rst
 F:	drivers/net/ethernet/huawei/hinic/
 
+HUAWEI MATEBOOK E GO EMBEDDED CONTROLLER DRIVER
+M:	Pengyu Luo <mitltlatltl@gmail.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/platform/huawei,gaokun-ec.yaml
+F:	drivers/platform/arm64/huawei-gaokun-ec.c
+F:	include/linux/platform_data/huawei-gaokun-ec.h
+
 HUGETLB SUBSYSTEM
 M:	Muchun Song <muchun.song@linux.dev>
 L:	linux-mm@kvack.org
diff --git a/drivers/platform/arm64/Kconfig b/drivers/platform/arm64/Kconfig
index f88395ea3..6ceee3c16 100644
--- a/drivers/platform/arm64/Kconfig
+++ b/drivers/platform/arm64/Kconfig
@@ -33,6 +33,27 @@ config EC_ACER_ASPIRE1
 	  laptop where this information is not properly exposed via the
 	  standard ACPI devices.
 
+config EC_HUAWEI_GAOKUN
+	tristate "Huawei Matebook E Go Embedded Controller driver"
+	depends on ARCH_QCOM || COMPILE_TEST
+	depends on I2C
+	depends on INPUT
+	select AUXILIARY_BUS
+
+	help
+	  Say Y here to enable the EC driver for the Huawei Matebook E Go
+	  which is a sc8280xp-based 2-in-1 tablet. The driver handles battery
+	  (information, charge control) and USB Type-C DP HPD events as well
+	  as some misc functions like the lid sensor and temperature sensors,
+	  etc.
+
+	  This driver provides battery and AC status support for the mentioned
+	  laptop where this information is not properly exposed via the
+	  standard ACPI devices.
+
+	  Say M or Y here to include this support.
+
+
 config EC_LENOVO_YOGA_C630
 	tristate "Lenovo Yoga C630 Embedded Controller driver"
 	depends on ARCH_QCOM || COMPILE_TEST
diff --git a/drivers/platform/arm64/Makefile b/drivers/platform/arm64/Makefile
index b2ae9114f..46a99eba3 100644
--- a/drivers/platform/arm64/Makefile
+++ b/drivers/platform/arm64/Makefile
@@ -6,4 +6,5 @@
 #
 
 obj-$(CONFIG_EC_ACER_ASPIRE1)	+= acer-aspire1-ec.o
+obj-$(CONFIG_EC_HUAWEI_GAOKUN)	+= huawei-gaokun-ec.o
 obj-$(CONFIG_EC_LENOVO_YOGA_C630) += lenovo-yoga-c630.o
diff --git a/drivers/platform/arm64/huawei-gaokun-ec.c b/drivers/platform/arm64/huawei-gaokun-ec.c
new file mode 100644
index 000000000..5b09b7d7c
--- /dev/null
+++ b/drivers/platform/arm64/huawei-gaokun-ec.c
@@ -0,0 +1,787 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * huawei-gaokun-ec - An EC driver for HUAWEI Matebook E Go
+ *
+ * Copyright (C) 2024-2025 Pengyu Luo <mitltlatltl@gmail.com>
+ */
+
+#include <linux/auxiliary_bus.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/notifier.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_data/huawei-gaokun-ec.h>
+
+#define EC_EVENT		0x06
+
+/* Also can be found in ACPI specification 12.3 */
+#define EC_READ			0x80
+#define EC_WRITE		0x81
+#define EC_BURST		0x82
+#define EC_QUERY		0x84
+
+#define EC_FN_LOCK_ON		0x5A
+#define EC_FN_LOCK_OFF		0x55
+
+#define EC_EVENT_LID		0x81
+
+#define EC_LID_STATE		0x80
+#define EC_LID_OPEN		BIT(1)
+
+#define UCSI_REG_SIZE		7
+
+/*
+ * For tx, command sequences are arranged as
+ * {master_cmd, slave_cmd, data_len, data_seq}
+ */
+#define REQ_HDR_SIZE		3
+#define INPUT_SIZE_OFFSET	2
+#define REQ_LEN(req) (REQ_HDR_SIZE + req[INPUT_SIZE_OFFSET])
+
+/*
+ * For rx, data sequences are arranged as
+ * {status, data_len(unreliable), data_seq}
+ */
+#define RESP_HDR_SIZE		2
+
+#define MKREQ(REG0, REG1, SIZE, ...)			\
+{							\
+	REG0, REG1, SIZE,				\
+	/* ## will remove comma when SIZE is 0 */	\
+	## __VA_ARGS__,					\
+	/* make sure len(pkt[3:]) >= SIZE */		\
+	[3 + SIZE] = 0,					\
+}
+
+#define MKRESP(SIZE)				\
+{						\
+	[RESP_HDR_SIZE + SIZE - 1] = 0,		\
+}
+
+/* Possible size 1, 4, 20, 24. Most of the time, the size is 1. */
+static inline void refill_req(u8 *dest, const u8 *src, size_t size)
+{
+	memcpy(dest + REQ_HDR_SIZE, src, size);
+}
+
+static inline void refill_req_byte(u8 *dest, const u8 *src)
+{
+	dest[REQ_HDR_SIZE] = *src;
+}
+
+/* Possible size 1, 2, 4, 7, 20. Most of the time, the size is 1. */
+static inline void extr_resp(u8 *dest, const u8 *src, size_t size)
+{
+	memcpy(dest, src + RESP_HDR_SIZE, size);
+}
+
+static inline void extr_resp_byte(u8 *dest, const u8 *src)
+{
+	*dest = src[RESP_HDR_SIZE];
+}
+
+static inline void *extr_resp_shallow(const u8 *src)
+{
+	return src + RESP_HDR_SIZE;
+}
+
+struct gaokun_ec {
+	struct i2c_client *client;
+	struct mutex lock; /* EC transaction lock */
+	struct blocking_notifier_head notifier_list;
+	struct device *hwmon_dev;
+	struct input_dev *idev;
+	bool suspended;
+};
+
+static int gaokun_ec_request(struct gaokun_ec *ec, const u8 *req,
+			     size_t resp_len, u8 *resp)
+{
+	struct i2c_client *client = ec->client;
+	struct i2c_msg msgs[2] = {
+		{
+			.addr = client->addr,
+			.flags = client->flags,
+			.len = REQ_LEN(req),
+			.buf = req,
+		}, {
+			.addr = client->addr,
+			.flags = client->flags | I2C_M_RD,
+			.len = resp_len,
+			.buf = resp,
+		},
+	};
+
+	guard(mutex)(&ec->lock);
+	i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+	usleep_range(2000, 2500); /* have a break, ACPI did this */
+
+	return *resp ? -EIO : 0;
+}
+
+/* -------------------------------------------------------------------------- */
+/* Common API */
+
+/**
+ * gaokun_ec_read - Read from EC
+ * @ec: The gaokun_ec structure
+ * @req: The sequence to request
+ * @resp_len: The size to read
+ * @resp: The buffer to store response sequence
+ *
+ * This function is used to read data after writing a magic sequence to EC.
+ * All EC operations depend on this function.
+ *
+ * Huawei uses magic sequences everywhere to complete various functions, all
+ * these sequences are passed to ECCD(a ACPI method which is quiet similar
+ * to gaokun_ec_request), there is no good abstraction to generalize these
+ * sequences, so just wrap it for now. Almost all magic sequences are kept
+ * in this file.
+ *
+ * Return: 0 on success or negative error code.
+ */
+int gaokun_ec_read(struct gaokun_ec *ec, const u8 *req,
+		   size_t resp_len, u8 *resp)
+{
+	return gaokun_ec_request(ec, req, resp_len, resp);
+}
+EXPORT_SYMBOL_GPL(gaokun_ec_read);
+
+/**
+ * gaokun_ec_write - Write to EC
+ * @ec: The gaokun_ec structure
+ * @req: The sequence to request
+ *
+ * This function has no big difference from gaokun_ec_read. When caller care
+ * only write status and no actual data are returned, then use it.
+ *
+ * Return: 0 on success or negative error code.
+ */
+int gaokun_ec_write(struct gaokun_ec *ec, const u8 *req)
+{
+	u8 ec_resp[] = MKRESP(0);
+
+	return gaokun_ec_request(ec, req, sizeof(ec_resp), ec_resp);
+}
+EXPORT_SYMBOL_GPL(gaokun_ec_write);
+
+int gaokun_ec_read_byte(struct gaokun_ec *ec, const u8 *req, u8 *byte)
+{
+	int ret;
+	u8 ec_resp[] = MKRESP(sizeof(*byte));
+
+	ret = gaokun_ec_read(ec, req, sizeof(ec_resp), ec_resp);
+	extr_resp_byte(byte, ec_resp);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(gaokun_ec_read_byte);
+
+/**
+ * gaokun_ec_register_notify - Register a notifier callback for EC events.
+ * @ec: The gaokun_ec structure
+ * @nb: Notifier block pointer to register
+ *
+ * Return: 0 on success or negative error code.
+ */
+int gaokun_ec_register_notify(struct gaokun_ec *ec, struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&ec->notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(gaokun_ec_register_notify);
+
+/**
+ * gaokun_ec_unregister_notify - Unregister notifier callback for EC events.
+ * @ec: The gaokun_ec structure
+ * @nb: Notifier block pointer to unregister
+ *
+ * Unregister a notifier callback that was previously registered with
+ * gaokun_ec_register_notify().
+ */
+void gaokun_ec_unregister_notify(struct gaokun_ec *ec, struct notifier_block *nb)
+{
+	blocking_notifier_chain_unregister(&ec->notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(gaokun_ec_unregister_notify);
+
+/* -------------------------------------------------------------------------- */
+/* API for PSY */
+
+/**
+ * gaokun_ec_psy_multi_read - Read contiguous registers
+ * @ec: The gaokun_ec structure
+ * @reg: The start register
+ * @resp_len: The number of registers to be read
+ * @resp: The buffer to store response sequence
+ *
+ * Return: 0 on success or negative error code.
+ */
+int gaokun_ec_psy_multi_read(struct gaokun_ec *ec, u8 reg,
+			     size_t resp_len, u8 *resp)
+{
+	u8 ec_req[] = MKREQ(0x02, EC_READ, 1, 0);
+	u8 ec_resp[] = MKRESP(1);
+	int i, ret;
+
+	for (i = 0; i < resp_len; ++i, reg++) {
+		refill_req_byte(ec_req, &reg);
+		ret = gaokun_ec_read(ec, ec_req, sizeof(ec_resp), ec_resp);
+		if (ret)
+			return ret;
+		extr_resp_byte(&resp[i], ec_resp);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gaokun_ec_psy_multi_read);
+
+/* Smart charge */
+
+/**
+ * gaokun_ec_psy_get_smart_charge - Get smart charge data from EC
+ * @ec: The gaokun_ec structure
+ * @resp: The buffer to store response sequence (mode, delay, start, end)
+ *
+ * Return: 0 on success or negative error code.
+ */
+int gaokun_ec_psy_get_smart_charge(struct gaokun_ec *ec,
+				   u8 resp[GAOKUN_SMART_CHARGE_DATA_SIZE])
+{
+	/* GBCM */
+	u8 ec_req[] = MKREQ(0x02, 0xE4, 0);
+	u8 ec_resp[] = MKRESP(GAOKUN_SMART_CHARGE_DATA_SIZE);
+	int ret;
+
+	ret = gaokun_ec_read(ec, ec_req, sizeof(ec_resp), ec_resp);
+	if (ret)
+		return ret;
+
+	extr_resp(resp, ec_resp, GAOKUN_SMART_CHARGE_DATA_SIZE);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gaokun_ec_psy_get_smart_charge);
+
+static inline bool are_thresholds_valid(u8 start, u8 end)
+{
+	return end != 0 && start <= end && end <= 100;
+}
+
+/**
+ * gaokun_ec_psy_set_smart_charge - Set smart charge data
+ * @ec: The gaokun_ec structure
+ * @req: The sequence to request (mode, delay, start, end)
+ *
+ * Return: 0 on success or negative error code.
+ */
+int gaokun_ec_psy_set_smart_charge(struct gaokun_ec *ec,
+				   const u8 req[GAOKUN_SMART_CHARGE_DATA_SIZE])
+{
+	/* SBCM */
+	u8 ec_req[] = MKREQ(0x02, 0xE3, GAOKUN_SMART_CHARGE_DATA_SIZE);
+
+	if (!are_thresholds_valid(req[2], req[3]))
+		return -EINVAL;
+
+	refill_req(ec_req, req, GAOKUN_SMART_CHARGE_DATA_SIZE);
+
+	return gaokun_ec_write(ec, ec_req);
+}
+EXPORT_SYMBOL_GPL(gaokun_ec_psy_set_smart_charge);
+
+/* Smart charge enable */
+
+/**
+ * gaokun_ec_psy_get_smart_charge_enable - Get smart charge state
+ * @ec: The gaokun_ec structure
+ * @on: The state
+ *
+ * Return: 0 on success or negative error code.
+ */
+int gaokun_ec_psy_get_smart_charge_enable(struct gaokun_ec *ec, bool *on)
+{
+	/* GBAC */
+	u8 ec_req[] = MKREQ(0x02, 0xE6, 0);
+	u8 state;
+	int ret;
+
+	ret = gaokun_ec_read_byte(ec, ec_req, &state);
+	if (ret)
+		return ret;
+
+	*on = !!state;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gaokun_ec_psy_get_smart_charge_enable);
+
+/**
+ * gaokun_ec_psy_set_smart_charge_enable - Set smart charge state
+ * @ec: The gaokun_ec structure
+ * @on: The state
+ *
+ * Return: 0 on success or negative error code.
+ */
+int gaokun_ec_psy_set_smart_charge_enable(struct gaokun_ec *ec, bool on)
+{
+	/* SBAC */
+	u8 ec_req[] = MKREQ(0x02, 0xE5, 1, on);
+
+	return gaokun_ec_write(ec, ec_req);
+}
+EXPORT_SYMBOL_GPL(gaokun_ec_psy_set_smart_charge_enable);
+
+/* -------------------------------------------------------------------------- */
+/* API for UCSI */
+
+/**
+ * gaokun_ec_ucsi_read - Read UCSI data from EC
+ * @ec: The gaokun_ec structure
+ * @resp: The buffer to store response sequence
+ *
+ * Read CCI and MSGI (used by UCSI subdriver).
+ *
+ * Return: 0 on success or negative error code.
+ */
+int gaokun_ec_ucsi_read(struct gaokun_ec *ec,
+			u8 resp[GAOKUN_UCSI_READ_SIZE])
+{
+	u8 ec_req[] = MKREQ(0x03, 0xD5, 0);
+	u8 ec_resp[] = MKRESP(GAOKUN_UCSI_READ_SIZE);
+	int ret;
+
+	ret = gaokun_ec_read(ec, ec_req, sizeof(ec_resp), ec_resp);
+	if (ret)
+		return ret;
+
+	extr_resp(resp, ec_resp, GAOKUN_UCSI_READ_SIZE);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gaokun_ec_ucsi_read);
+
+/**
+ * gaokun_ec_ucsi_write - Write UCSI data to EC
+ * @ec: The gaokun_ec structure
+ * @req: The sequence to request
+ *
+ * Write CTRL and MSGO (used by UCSI subdriver).
+ *
+ * Return: 0 on success or negative error code.
+ */
+int gaokun_ec_ucsi_write(struct gaokun_ec *ec,
+			 const u8 req[GAOKUN_UCSI_WRITE_SIZE])
+{
+	u8 ec_req[] = MKREQ(0x03, 0xD4, GAOKUN_UCSI_WRITE_SIZE);
+
+	refill_req(ec_req, req, GAOKUN_UCSI_WRITE_SIZE);
+
+	return gaokun_ec_write(ec, ec_req);
+}
+EXPORT_SYMBOL_GPL(gaokun_ec_ucsi_write);
+
+/**
+ * gaokun_ec_ucsi_get_reg - Get UCSI register from EC
+ * @ec: The gaokun_ec structure
+ * @ureg: The gaokun ucsi register
+ *
+ * Get UCSI register data (used by UCSI subdriver).
+ *
+ * Return: 0 on success or negative error code.
+ */
+int gaokun_ec_ucsi_get_reg(struct gaokun_ec *ec, struct gaokun_ucsi_reg *ureg)
+{
+	u8 ec_req[] = MKREQ(0x03, 0xD3, 0);
+	u8 ec_resp[] = MKRESP(UCSI_REG_SIZE);
+	int ret;
+
+	ret = gaokun_ec_read(ec, ec_req, sizeof(ec_resp), ec_resp);
+	if (ret)
+		return ret;
+
+	extr_resp((u8 *)ureg, ec_resp, UCSI_REG_SIZE);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gaokun_ec_ucsi_get_reg);
+
+/**
+ * gaokun_ec_ucsi_pan_ack - Ack pin assignment notifications from EC
+ * @ec: The gaokun_ec structure
+ * @port_id: The port id receiving and handling the notifications
+ *
+ * Ack pin assignment notifications (used by UCSI subdriver).
+ *
+ * Return: 0 on success or negative error code.
+ */
+int gaokun_ec_ucsi_pan_ack(struct gaokun_ec *ec, int port_id)
+{
+	u8 ec_req[] = MKREQ(0x03, 0xD2, 1);
+	u8 data = 1 << port_id;
+
+	if (port_id == GAOKUN_UCSI_NO_PORT_UPDATE)
+		data = 0;
+
+	refill_req_byte(ec_req, &data);
+
+	return gaokun_ec_write(ec, ec_req);
+}
+EXPORT_SYMBOL_GPL(gaokun_ec_ucsi_pan_ack);
+
+/* -------------------------------------------------------------------------- */
+/* EC Sysfs */
+
+/* Fn lock */
+static int gaokun_ec_get_fn_lock(struct gaokun_ec *ec, bool *on)
+{
+	/* GFRS */
+	u8 ec_req[] = MKREQ(0x02, 0x6B, 0);
+	int ret;
+	u8 state;
+
+	ret = gaokun_ec_read_byte(ec, ec_req, &state);
+	if (ret)
+		return ret;
+
+	if (state == EC_FN_LOCK_ON)
+		*on = true;
+	else if (state == EC_FN_LOCK_OFF)
+		*on = false;
+	else
+		return -EIO;
+
+	return 0;
+}
+
+static int gaokun_ec_set_fn_lock(struct gaokun_ec *ec, bool on)
+{
+	/* SFRS */
+	u8 ec_req[] = MKREQ(0x02, 0x6C, 1, on ? EC_FN_LOCK_ON : EC_FN_LOCK_OFF);
+
+	return gaokun_ec_write(ec, ec_req);
+}
+
+static ssize_t fn_lock_show(struct device *dev,
+			    struct device_attribute *attr,
+			    char *buf)
+{
+	struct gaokun_ec *ec = dev_get_drvdata(dev);
+	bool on;
+	int ret;
+
+	ret = gaokun_ec_get_fn_lock(ec, &on);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%d\n", on);
+}
+
+static ssize_t fn_lock_store(struct device *dev,
+			     struct device_attribute *attr,
+			     const char *buf, size_t size)
+{
+	struct gaokun_ec *ec = dev_get_drvdata(dev);
+	bool on;
+	int ret;
+
+	if (kstrtobool(buf, &on))
+		return -EINVAL;
+
+	ret = gaokun_ec_set_fn_lock(ec, on);
+	if (ret)
+		return ret;
+
+	return size;
+}
+
+static DEVICE_ATTR_RW(fn_lock);
+
+static struct attribute *gaokun_ec_attrs[] = {
+	&dev_attr_fn_lock.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(gaokun_ec);
+
+/* -------------------------------------------------------------------------- */
+/* Thermal Zone HwMon */
+
+/* Range from 0 to 0x2C, partially valid */
+static const u8 temp_reg[] = {
+	0x05, 0x07, 0x08, 0x0E, 0x0F, 0x12, 0x15, 0x1E,
+	0x1F, 0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26,
+	0x27, 0x28, 0x29, 0x2A
+};
+
+static int gaokun_ec_get_temp(struct gaokun_ec *ec, u8 idx, long *temp)
+{
+	/* GTMP */
+	u8 ec_req[] = MKREQ(0x02, 0x61, 1, temp_reg[idx]);
+	u8 ec_resp[] = MKRESP(sizeof(__le16));
+	__le16 *tmp;
+	int ret;
+
+	ret = gaokun_ec_read(ec, ec_req, sizeof(ec_resp), ec_resp);
+	if (ret)
+		return ret;
+
+	tmp = (__le16 *)extr_resp_shallow(ec_resp);
+	*temp = le16_to_cpu(*tmp) * 100; /* convert to HwMon's unit */
+
+	return 0;
+}
+
+static umode_t
+gaokun_ec_hwmon_is_visible(const void *data, enum hwmon_sensor_types type,
+			   u32 attr, int channel)
+{
+	return type == hwmon_temp ? 0444 : 0;
+}
+
+static int
+gaokun_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+		     u32 attr, int channel, long *val)
+{
+	struct gaokun_ec *ec = dev_get_drvdata(dev);
+
+	if (type == hwmon_temp)
+		return gaokun_ec_get_temp(ec, channel, val);
+
+	return -EINVAL;
+}
+
+static const struct hwmon_ops gaokun_ec_hwmon_ops = {
+	.is_visible = gaokun_ec_hwmon_is_visible,
+	.read = gaokun_ec_hwmon_read,
+};
+
+static u32 gaokun_ec_temp_config[] = {
+	[0 ... ARRAY_SIZE(temp_reg) - 1] = HWMON_T_INPUT,
+	0
+};
+
+static const struct hwmon_channel_info gaokun_ec_temp = {
+	.type = hwmon_temp,
+	.config = gaokun_ec_temp_config,
+};
+
+static const struct hwmon_channel_info * const gaokun_ec_hwmon_info[] = {
+	&gaokun_ec_temp,
+	NULL
+};
+
+static const struct hwmon_chip_info gaokun_ec_hwmon_chip_info = {
+	.ops = &gaokun_ec_hwmon_ops,
+	.info = gaokun_ec_hwmon_info,
+};
+
+/* -------------------------------------------------------------------------- */
+/* Modern Standby */
+
+static int gaokun_ec_suspend(struct device *dev)
+{
+	struct gaokun_ec *ec = dev_get_drvdata(dev);
+	u8 ec_req[] = MKREQ(0x02, 0xB2, 1, 0xDB);
+	int ret;
+
+	if (ec->suspended)
+		return 0;
+
+	ret = gaokun_ec_write(ec, ec_req);
+	if (ret)
+		return ret;
+
+	ec->suspended = true;
+
+	return 0;
+}
+
+static int gaokun_ec_resume(struct device *dev)
+{
+	struct gaokun_ec *ec = dev_get_drvdata(dev);
+	u8 ec_req[] = MKREQ(0x02, 0xB2, 1, 0xEB);
+	int ret;
+	int i;
+
+	if (!ec->suspended)
+		return 0;
+
+	for (i = 0; i < 3; ++i) {
+		ret = gaokun_ec_write(ec, ec_req);
+		if (ret == 0)
+			break;
+
+		msleep(100); /* EC need time to resume */
+	};
+
+	ec->suspended = false;
+
+	return 0;
+}
+
+static void gaokun_aux_release(struct device *dev)
+{
+	struct auxiliary_device *adev = to_auxiliary_dev(dev);
+
+	kfree(adev);
+}
+
+static void gaokun_aux_remove(void *data)
+{
+	struct auxiliary_device *adev = data;
+
+	auxiliary_device_delete(adev);
+	auxiliary_device_uninit(adev);
+}
+
+static int gaokun_aux_init(struct device *parent, const char *name,
+			   struct gaokun_ec *ec)
+{
+	struct auxiliary_device *adev;
+	int ret;
+
+	adev = kzalloc(sizeof(*adev), GFP_KERNEL);
+	if (!adev)
+		return -ENOMEM;
+
+	adev->name = name;
+	adev->id = 0;
+	adev->dev.parent = parent;
+	adev->dev.release = gaokun_aux_release;
+	adev->dev.platform_data = ec;
+	/* Allow aux devices to access parent's DT nodes directly */
+	device_set_of_node_from_dev(&adev->dev, parent);
+
+	ret = auxiliary_device_init(adev);
+	if (ret) {
+		kfree(adev);
+		return ret;
+	}
+
+	ret = auxiliary_device_add(adev);
+	if (ret) {
+		auxiliary_device_uninit(adev);
+		return ret;
+	}
+
+	return devm_add_action_or_reset(parent, gaokun_aux_remove, adev);
+}
+
+/* -------------------------------------------------------------------------- */
+/* EC */
+
+static irqreturn_t gaokun_ec_irq_handler(int irq, void *data)
+{
+	struct gaokun_ec *ec = data;
+	u8 ec_req[] = MKREQ(EC_EVENT, EC_QUERY, 0);
+	u8 status, id;
+	int ret;
+
+	ret = gaokun_ec_read_byte(ec, ec_req, &id);
+	if (ret)
+		return IRQ_HANDLED;
+
+	switch (id) {
+	case 0x0: /* No event */
+		break;
+
+	case EC_EVENT_LID:
+		gaokun_ec_psy_read_byte(ec, EC_LID_STATE, &status);
+		status = EC_LID_OPEN & status;
+		input_report_switch(ec->idev, SW_LID, !status);
+		input_sync(ec->idev);
+		break;
+
+	default:
+		blocking_notifier_call_chain(&ec->notifier_list, id, ec);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int gaokun_ec_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct gaokun_ec *ec;
+	int ret;
+
+	ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
+	if (!ec)
+		return -ENOMEM;
+
+	mutex_init(&ec->lock);
+	ec->client = client;
+	i2c_set_clientdata(client, ec);
+	BLOCKING_INIT_NOTIFIER_HEAD(&ec->notifier_list);
+
+	/* Lid switch */
+	ec->idev = devm_input_allocate_device(dev);
+	if (!ec->idev)
+		return -ENOMEM;
+
+	ec->idev->name = "LID";
+	ec->idev->phys = "gaokun-ec/input0";
+	input_set_capability(ec->idev, EV_SW, SW_LID);
+
+	ret = input_register_device(ec->idev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to register input device\n");
+
+	ret = gaokun_aux_init(dev, GAOKUN_DEV_PSY, ec);
+	if (ret)
+		return ret;
+
+	ret = gaokun_aux_init(dev, GAOKUN_DEV_UCSI, ec);
+	if (ret)
+		return ret;
+
+	ret = devm_request_threaded_irq(dev, client->irq, NULL,
+					gaokun_ec_irq_handler, IRQF_ONESHOT,
+					dev_name(dev), ec);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to request IRQ\n");
+
+	ec->hwmon_dev = devm_hwmon_device_register_with_info(dev, "gaokun_ec_hwmon",
+							     ec, &gaokun_ec_hwmon_chip_info, NULL);
+	if (IS_ERR(ec->hwmon_dev))
+		return dev_err_probe(dev, PTR_ERR(ec->hwmon_dev),
+				     "Failed to register hwmon device\n");
+
+	return 0;
+}
+
+static const struct i2c_device_id gaokun_ec_id[] = {
+	{ "gaokun-ec", },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, gaokun_ec_id);
+
+static const struct of_device_id gaokun_ec_of_match[] = {
+	{ .compatible = "huawei,gaokun3-ec", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, gaokun_ec_of_match);
+
+static const struct dev_pm_ops gaokun_ec_pm_ops = {
+	NOIRQ_SYSTEM_SLEEP_PM_OPS(gaokun_ec_suspend, gaokun_ec_resume)
+};
+
+static struct i2c_driver gaokun_ec_driver = {
+	.driver = {
+		.name = "gaokun-ec",
+		.of_match_table = gaokun_ec_of_match,
+		.pm = &gaokun_ec_pm_ops,
+		.dev_groups = gaokun_ec_groups,
+	},
+	.probe = gaokun_ec_probe,
+	.id_table = gaokun_ec_id,
+};
+module_i2c_driver(gaokun_ec_driver);
+
+MODULE_DESCRIPTION("HUAWEI Matebook E Go EC driver");
+MODULE_AUTHOR("Pengyu Luo <mitltlatltl@gmail.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/platform_data/huawei-gaokun-ec.h b/include/linux/platform_data/huawei-gaokun-ec.h
new file mode 100644
index 000000000..6a1e923d4
--- /dev/null
+++ b/include/linux/platform_data/huawei-gaokun-ec.h
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Huawei Matebook E Go Embedded Controller
+ *
+ * Copyright (C) 2024-2025 Pengyu Luo <mitltlatltl@gmail.com>
+ */
+
+#ifndef __HUAWEI_GAOKUN_EC_H__
+#define __HUAWEI_GAOKUN_EC_H__
+
+#define GAOKUN_UCSI_CCI_SIZE	4
+#define GAOKUN_UCSI_MSGI_SIZE	16
+#define GAOKUN_UCSI_READ_SIZE	(GAOKUN_UCSI_CCI_SIZE + GAOKUN_UCSI_MSGI_SIZE)
+#define GAOKUN_UCSI_WRITE_SIZE	24 /* 8B CTRL, 16B MSGO */
+
+#define GAOKUN_UCSI_NO_PORT_UPDATE	(-1)
+
+#define GAOKUN_SMART_CHARGE_DATA_SIZE	4 /* mode, delay, start, end */
+
+/* -------------------------------------------------------------------------- */
+
+struct gaokun_ec;
+struct gaokun_ucsi_reg;
+struct notifier_block;
+
+#define GAOKUN_MOD_NAME			"huawei_gaokun_ec"
+#define GAOKUN_DEV_PSY			"psy"
+#define GAOKUN_DEV_UCSI			"ucsi"
+
+/* -------------------------------------------------------------------------- */
+/* Common API */
+
+int gaokun_ec_register_notify(struct gaokun_ec *ec,
+			      struct notifier_block *nb);
+void gaokun_ec_unregister_notify(struct gaokun_ec *ec,
+				 struct notifier_block *nb);
+
+int gaokun_ec_read(struct gaokun_ec *ec, const u8 *req,
+		   size_t resp_len, u8 *resp);
+int gaokun_ec_write(struct gaokun_ec *ec, const u8 *req);
+int gaokun_ec_read_byte(struct gaokun_ec *ec, const u8 *req, u8 *byte);
+
+/* -------------------------------------------------------------------------- */
+/* API for PSY */
+
+int gaokun_ec_psy_multi_read(struct gaokun_ec *ec, u8 reg,
+			     size_t resp_len, u8 *resp);
+
+static inline int gaokun_ec_psy_read_byte(struct gaokun_ec *ec,
+					  u8 reg, u8 *byte)
+{
+	return gaokun_ec_psy_multi_read(ec, reg, sizeof(*byte), byte);
+}
+
+static inline int gaokun_ec_psy_read_word(struct gaokun_ec *ec,
+					  u8 reg, u16 *word)
+{
+	return gaokun_ec_psy_multi_read(ec, reg, sizeof(*word), (u8 *)word);
+}
+
+int gaokun_ec_psy_get_smart_charge(struct gaokun_ec *ec,
+				   u8 resp[GAOKUN_SMART_CHARGE_DATA_SIZE]);
+int gaokun_ec_psy_set_smart_charge(struct gaokun_ec *ec,
+				   const u8 req[GAOKUN_SMART_CHARGE_DATA_SIZE]);
+
+int gaokun_ec_psy_get_smart_charge_enable(struct gaokun_ec *ec, bool *on);
+int gaokun_ec_psy_set_smart_charge_enable(struct gaokun_ec *ec, bool on);
+
+/* -------------------------------------------------------------------------- */
+/* API for UCSI */
+
+int gaokun_ec_ucsi_read(struct gaokun_ec *ec, u8 resp[GAOKUN_UCSI_READ_SIZE]);
+int gaokun_ec_ucsi_write(struct gaokun_ec *ec,
+			 const u8 req[GAOKUN_UCSI_WRITE_SIZE]);
+
+int gaokun_ec_ucsi_get_reg(struct gaokun_ec *ec, struct gaokun_ucsi_reg *ureg);
+int gaokun_ec_ucsi_pan_ack(struct gaokun_ec *ec, int port_id);
+
+
+#endif /* __HUAWEI_GAOKUN_EC_H__ */
-- 
2.48.1

[PATCH v4 3/3] arm64: dts: qcom: gaokun3: Add Embedded Controller node

[Pengyu Luo]

The Embedded Controller in the Huawei Matebook E Go is accessible on &i2c15
and provides battery and adapter status, port orientation status, as well
as HPD event notifications for two USB Type-C port, etc.

Add the EC to the device tree and describe the relationship among
the type-c connectors, role switches, orientation switches and the QMP
combo PHY.

Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
---
 .../boot/dts/qcom/sc8280xp-huawei-gaokun3.dts | 163 ++++++++++++++++++
 1 file changed, 163 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-huawei-gaokun3.dts b/arch/arm64/boot/dts/qcom/sc8280xp-huawei-gaokun3.dts
index 09b95f89e..1667c7157 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-huawei-gaokun3.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-huawei-gaokun3.dts
@@ -28,6 +28,7 @@ / {
 
 	aliases {
 		i2c4 = &i2c4;
+		i2c15 = &i2c15;
 		serial1 = &uart2;
 	};
 
@@ -216,6 +217,40 @@ map1 {
 		};
 	};
 
+	usb0-sbu-mux {
+		compatible = "pericom,pi3usb102", "gpio-sbu-mux";
+
+		select-gpios = <&tlmm 164 GPIO_ACTIVE_HIGH>;
+
+		pinctrl-0 = <&usb0_sbu_default>;
+		pinctrl-names = "default";
+
+		orientation-switch;
+
+		port {
+			usb0_sbu_mux: endpoint {
+				remote-endpoint = <&ucsi0_sbu>;
+			};
+		};
+	};
+
+	usb1-sbu-mux {
+		compatible = "pericom,pi3usb102", "gpio-sbu-mux";
+
+		select-gpios = <&tlmm 47 GPIO_ACTIVE_HIGH>;
+
+		pinctrl-0 = <&usb1_sbu_default>;
+		pinctrl-names = "default";
+
+		orientation-switch;
+
+		port {
+			usb1_sbu_mux: endpoint {
+				remote-endpoint = <&ucsi1_sbu>;
+			};
+		};
+	};
+
 	wcn6855-pmu {
 		compatible = "qcom,wcn6855-pmu";
 
@@ -584,6 +619,97 @@ touchscreen@4f {
 
 };
 
+&i2c15 {
+	clock-frequency = <400000>;
+
+	pinctrl-0 = <&i2c15_default>;
+	pinctrl-names = "default";
+
+	status = "okay";
+
+	embedded-controller@38 {
+		compatible = "huawei,gaokun3-ec";
+		reg = <0x38>;
+
+		interrupts-extended = <&tlmm 107 IRQ_TYPE_LEVEL_LOW>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		connector@0 {
+			compatible = "usb-c-connector";
+			reg = <0>;
+			power-role = "dual";
+			data-role = "dual";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+
+					ucsi0_hs_in: endpoint {
+						remote-endpoint = <&usb_0_dwc3_hs>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+
+					ucsi0_ss_in: endpoint {
+						remote-endpoint = <&usb_0_qmpphy_out>;
+					};
+				};
+
+				port@2 {
+					reg = <2>;
+
+					ucsi0_sbu: endpoint {
+						remote-endpoint = <&usb0_sbu_mux>;
+					};
+				};
+			};
+		};
+
+		connector@1 {
+			compatible = "usb-c-connector";
+			reg = <1>;
+			power-role = "dual";
+			data-role = "dual";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+
+					ucsi1_hs_in: endpoint {
+						remote-endpoint = <&usb_1_dwc3_hs>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+
+					ucsi1_ss_in: endpoint {
+						remote-endpoint = <&usb_1_qmpphy_out>;
+					};
+				};
+
+				port@2 {
+					reg = <2>;
+
+					ucsi1_sbu: endpoint {
+						remote-endpoint = <&usb1_sbu_mux>;
+					};
+				};
+			};
+		};
+	};
+};
+
 &mdss0 {
 	status = "okay";
 };
@@ -1004,6 +1130,10 @@ &usb_0_dwc3 {
 	dr_mode = "host";
 };
 
+&usb_0_dwc3_hs {
+	remote-endpoint = <&ucsi0_hs_in>;
+};
+
 &usb_0_hsphy {
 	vdda-pll-supply = <&vreg_l9d>;
 	vdda18-supply = <&vreg_l1c>;
@@ -1025,6 +1155,10 @@ &usb_0_qmpphy_dp_in {
 	remote-endpoint = <&mdss0_dp0_out>;
 };
 
+&usb_0_qmpphy_out {
+	remote-endpoint = <&ucsi0_ss_in>;
+};
+
 &usb_1 {
 	status = "okay";
 };
@@ -1033,6 +1167,10 @@ &usb_1_dwc3 {
 	dr_mode = "host";
 };
 
+&usb_1_dwc3_hs {
+	remote-endpoint = <&ucsi1_hs_in>;
+};
+
 &usb_1_hsphy {
 	vdda-pll-supply = <&vreg_l4b>;
 	vdda18-supply = <&vreg_l1c>;
@@ -1054,6 +1192,10 @@ &usb_1_qmpphy_dp_in {
 	remote-endpoint = <&mdss0_dp1_out>;
 };
 
+&usb_1_qmpphy_out {
+	remote-endpoint = <&ucsi1_ss_in>;
+};
+
 &usb_2 {
 	status = "okay";
 };
@@ -1177,6 +1319,13 @@ i2c4_default: i2c4-default-state {
 		bias-disable;
 	};
 
+	i2c15_default: i2c15-default-state {
+		pins = "gpio36", "gpio37";
+		function = "qup15";
+		drive-strength = <2>;
+		bias-pull-up;
+	};
+
 	mode_pin_active: mode-pin-state {
 		pins = "gpio26";
 		function = "gpio";
@@ -1301,6 +1450,20 @@ tx-pins {
 		};
 	};
 
+	usb0_sbu_default: usb0-sbu-state {
+		pins = "gpio164";
+		function = "gpio";
+		drive-strength = <16>;
+		bias-disable;
+	};
+
+	usb1_sbu_default: usb1-sbu-state {
+		pins = "gpio47";
+		function = "gpio";
+		drive-strength = <16>;
+		bias-disable;
+	};
+
 	wcd_default: wcd-default-state {
 		reset-pins {
 			pins = "gpio106";
-- 
2.48.1

Re: [PATCH v4 3/3] arm64: dts: qcom: gaokun3: Add Embedded Controller node

[Krzysztof Kozlowski]

On 16/01/2025 12:15, Pengyu Luo wrote:
> The Embedded Controller in the Huawei Matebook E Go is accessible on &i2c15
> and provides battery and adapter status, port orientation status, as well
> as HPD event notifications for two USB Type-C port, etc.
> 
> Add the EC to the device tree and describe the relationship among
> the type-c connectors, role switches, orientation switches and the QMP
> combo PHY.
> 
> Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
> ---
>  .../boot/dts/qcom/sc8280xp-huawei-gaokun3.dts | 163 ++++++++++++++++++


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v4 2/3] platform: arm64: add Huawei Matebook E Go EC driver

[Bryan O'Donoghue]

On 16/01/2025 11:15, Pengyu Luo wrote:
> There are three variants of which Huawei released the first two
> simultaneously.
> 
> Huawei Matebook E Go LTE(sc8180x), codename seems to be gaokun2.
> Huawei Matebook E Go(sc8280xp@3.0GHz), codename must be gaokun3. (see [1])
> Huawei Matebook E Go 2023(sc8280xp@2.69GHz), codename should be also gaokun3.
> 
> Adding support for the latter two variants for now, this driver should
> also work for the sc8180x variant according to acpi table files, but I
> don't have the device to test yet.
> 
> Different from other Qualcomm Snapdragon sc8280xp based machines, the
> Huawei Matebook E Go uses an embedded controller while others use
> a system called PMIC GLink. This embedded controller can be used to
> perform a set of various functions, including, but not limited to:
> 
> - Battery and charger monitoring;
> - Charge control and smart charge;
> - Fn_lock settings;
> - Tablet lid status;
> - Temperature sensors;
> - USB Type-C notifications (ports orientation,  DP alt mode HPD);
> - USB Type-C PD (according to observation, up to 48w).
> 
> Add a driver for the EC which creates devices for UCSI and power supply
> devices.
> 
> This driver is inspired by the following drivers:
>          drivers/platform/arm64/acer-aspire1-ec.c
>          drivers/platform/arm64/lenovo-yoga-c630.c
>          drivers/platform/x86/huawei-wmi.c
> 
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=219645
> 
> Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
> ---
>   MAINTAINERS                                   |   7 +
>   drivers/platform/arm64/Kconfig                |  21 +
>   drivers/platform/arm64/Makefile               |   1 +
>   drivers/platform/arm64/huawei-gaokun-ec.c     | 787 ++++++++++++++++++
>   .../linux/platform_data/huawei-gaokun-ec.h    |  80 ++
>   5 files changed, 896 insertions(+)
>   create mode 100644 drivers/platform/arm64/huawei-gaokun-ec.c
>   create mode 100644 include/linux/platform_data/huawei-gaokun-ec.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 33b9cd11a..27ff24e7d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10692,6 +10692,13 @@ S:	Maintained
>   F:	Documentation/networking/device_drivers/ethernet/huawei/hinic.rst
>   F:	drivers/net/ethernet/huawei/hinic/
>   
> +HUAWEI MATEBOOK E GO EMBEDDED CONTROLLER DRIVER
> +M:	Pengyu Luo <mitltlatltl@gmail.com>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/platform/huawei,gaokun-ec.yaml
> +F:	drivers/platform/arm64/huawei-gaokun-ec.c
> +F:	include/linux/platform_data/huawei-gaokun-ec.h
> +
>   HUGETLB SUBSYSTEM
>   M:	Muchun Song <muchun.song@linux.dev>
>   L:	linux-mm@kvack.org
> diff --git a/drivers/platform/arm64/Kconfig b/drivers/platform/arm64/Kconfig
> index f88395ea3..6ceee3c16 100644
> --- a/drivers/platform/arm64/Kconfig
> +++ b/drivers/platform/arm64/Kconfig
> @@ -33,6 +33,27 @@ config EC_ACER_ASPIRE1
>   	  laptop where this information is not properly exposed via the
>   	  standard ACPI devices.
>   
> +config EC_HUAWEI_GAOKUN
> +	tristate "Huawei Matebook E Go Embedded Controller driver"
> +	depends on ARCH_QCOM || COMPILE_TEST
> +	depends on I2C
> +	depends on INPUT
> +	select AUXILIARY_BUS
> +
> +	help
> +	  Say Y here to enable the EC driver for the Huawei Matebook E Go
> +	  which is a sc8280xp-based 2-in-1 tablet. The driver handles battery
> +	  (information, charge control) and USB Type-C DP HPD events as well
> +	  as some misc functions like the lid sensor and temperature sensors,
> +	  etc.
> +
> +	  This driver provides battery and AC status support for the mentioned
> +	  laptop where this information is not properly exposed via the
> +	  standard ACPI devices.
> +
> +	  Say M or Y here to include this support.
> +
> +
>   config EC_LENOVO_YOGA_C630
>   	tristate "Lenovo Yoga C630 Embedded Controller driver"
>   	depends on ARCH_QCOM || COMPILE_TEST
> diff --git a/drivers/platform/arm64/Makefile b/drivers/platform/arm64/Makefile
> index b2ae9114f..46a99eba3 100644
> --- a/drivers/platform/arm64/Makefile
> +++ b/drivers/platform/arm64/Makefile
> @@ -6,4 +6,5 @@
>   #
>   
>   obj-$(CONFIG_EC_ACER_ASPIRE1)	+= acer-aspire1-ec.o
> +obj-$(CONFIG_EC_HUAWEI_GAOKUN)	+= huawei-gaokun-ec.o
>   obj-$(CONFIG_EC_LENOVO_YOGA_C630) += lenovo-yoga-c630.o
> diff --git a/drivers/platform/arm64/huawei-gaokun-ec.c b/drivers/platform/arm64/huawei-gaokun-ec.c
> new file mode 100644
> index 000000000..5b09b7d7c
> --- /dev/null
> +++ b/drivers/platform/arm64/huawei-gaokun-ec.c
> @@ -0,0 +1,787 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * huawei-gaokun-ec - An EC driver for HUAWEI Matebook E Go
> + *
> + * Copyright (C) 2024-2025 Pengyu Luo <mitltlatltl@gmail.com>
> + */
> +
> +#include <linux/auxiliary_bus.h>
> +#include <linux/cleanup.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/notifier.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_data/huawei-gaokun-ec.h>
> +
> +#define EC_EVENT		0x06
> +
> +/* Also can be found in ACPI specification 12.3 */
> +#define EC_READ			0x80
> +#define EC_WRITE		0x81
> +#define EC_BURST		0x82
> +#define EC_QUERY		0x84
> +
> +#define EC_FN_LOCK_ON		0x5A
> +#define EC_FN_LOCK_OFF		0x55
> +
> +#define EC_EVENT_LID		0x81
> +
> +#define EC_LID_STATE		0x80
> +#define EC_LID_OPEN		BIT(1)
> +
> +#define UCSI_REG_SIZE		7
> +
> +/*
> + * For tx, command sequences are arranged as
> + * {master_cmd, slave_cmd, data_len, data_seq}
> + */
> +#define REQ_HDR_SIZE		3
> +#define INPUT_SIZE_OFFSET	2
> +#define REQ_LEN(req) (REQ_HDR_SIZE + req[INPUT_SIZE_OFFSET])
> +
> +/*
> + * For rx, data sequences are arranged as
> + * {status, data_len(unreliable), data_seq}
> + */
> +#define RESP_HDR_SIZE		2
> +
> +#define MKREQ(REG0, REG1, SIZE, ...)			\
> +{							\
> +	REG0, REG1, SIZE,				\
> +	/* ## will remove comma when SIZE is 0 */	\
> +	## __VA_ARGS__,					\
> +	/* make sure len(pkt[3:]) >= SIZE */		\
> +	[3 + SIZE] = 0,					\
> +}
> +
> +#define MKRESP(SIZE)				\
> +{						\
> +	[RESP_HDR_SIZE + SIZE - 1] = 0,		\
> +}
> +
> +/* Possible size 1, 4, 20, 24. Most of the time, the size is 1. */
> +static inline void refill_req(u8 *dest, const u8 *src, size_t size)
> +{
> +	memcpy(dest + REQ_HDR_SIZE, src, size);
> +}
> +
> +static inline void refill_req_byte(u8 *dest, const u8 *src)
> +{
> +	dest[REQ_HDR_SIZE] = *src;
> +}
> +
> +/* Possible size 1, 2, 4, 7, 20. Most of the time, the size is 1. */
> +static inline void extr_resp(u8 *dest, const u8 *src, size_t size)
> +{
> +	memcpy(dest, src + RESP_HDR_SIZE, size);
> +}
> +
> +static inline void extr_resp_byte(u8 *dest, const u8 *src)
> +{
> +	*dest = src[RESP_HDR_SIZE];
> +}
> +
> +static inline void *extr_resp_shallow(const u8 *src)
> +{
> +	return src + RESP_HDR_SIZE;
> +}
> +
> +struct gaokun_ec {
> +	struct i2c_client *client;
> +	struct mutex lock; /* EC transaction lock */
> +	struct blocking_notifier_head notifier_list;
> +	struct device *hwmon_dev;
> +	struct input_dev *idev;
> +	bool suspended;
> +};
> +
> +static int gaokun_ec_request(struct gaokun_ec *ec, const u8 *req,
> +			     size_t resp_len, u8 *resp)
> +{
> +	struct i2c_client *client = ec->client;
> +	struct i2c_msg msgs[2] = {
> +		{
> +			.addr = client->addr,
> +			.flags = client->flags,
> +			.len = REQ_LEN(req),
> +			.buf = req,
> +		}, {
> +			.addr = client->addr,
> +			.flags = client->flags | I2C_M_RD,
> +			.len = resp_len,
> +			.buf = resp,
> +		},
> +	};
> +
> +	guard(mutex)(&ec->lock);
> +	i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));

You should trap the result code of i2c_transfer() and push it up the 
call stack.

> +	usleep_range(2000, 2500); /* have a break, ACPI did this */
> +
> +	return *resp ? -EIO : 0;

If the value @ *resp is non-zero return -EIO ?

Why ?

> +}
> +
> +/* -------------------------------------------------------------------------- */
> +/* Common API */
> +
> +/**
> + * gaokun_ec_read - Read from EC
> + * @ec: The gaokun_ec structure
> + * @req: The sequence to request
> + * @resp_len: The size to read
> + * @resp: The buffer to store response sequence
> + *
> + * This function is used to read data after writing a magic sequence to EC.
> + * All EC operations depend on this function.
> + *
> + * Huawei uses magic sequences everywhere to complete various functions, all
> + * these sequences are passed to ECCD(a ACPI method which is quiet similar
> + * to gaokun_ec_request), there is no good abstraction to generalize these
> + * sequences, so just wrap it for now. Almost all magic sequences are kept
> + * in this file.
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int gaokun_ec_read(struct gaokun_ec *ec, const u8 *req,
> +		   size_t resp_len, u8 *resp)
> +{
> +	return gaokun_ec_request(ec, req, resp_len, resp);
> +}
> +EXPORT_SYMBOL_GPL(gaokun_ec_read);
> +
> +/**
> + * gaokun_ec_write - Write to EC
> + * @ec: The gaokun_ec structure
> + * @req: The sequence to request
> + *
> + * This function has no big difference from gaokun_ec_read. When caller care
> + * only write status and no actual data are returned, then use it.
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int gaokun_ec_write(struct gaokun_ec *ec, const u8 *req)
> +{
> +	u8 ec_resp[] = MKRESP(0);
> +
> +	return gaokun_ec_request(ec, req, sizeof(ec_resp), ec_resp);
> +}
> +EXPORT_SYMBOL_GPL(gaokun_ec_write);
> +
> +int gaokun_ec_read_byte(struct gaokun_ec *ec, const u8 *req, u8 *byte)
> +{
> +	int ret;
> +	u8 ec_resp[] = MKRESP(sizeof(*byte));
> +
> +	ret = gaokun_ec_read(ec, req, sizeof(ec_resp), ec_resp);
> +	extr_resp_byte(byte, ec_resp);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(gaokun_ec_read_byte);
> +
> +/**
> + * gaokun_ec_register_notify - Register a notifier callback for EC events.
> + * @ec: The gaokun_ec structure
> + * @nb: Notifier block pointer to register
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int gaokun_ec_register_notify(struct gaokun_ec *ec, struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_register(&ec->notifier_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(gaokun_ec_register_notify);
> +
> +/**
> + * gaokun_ec_unregister_notify - Unregister notifier callback for EC events.
> + * @ec: The gaokun_ec structure
> + * @nb: Notifier block pointer to unregister
> + *
> + * Unregister a notifier callback that was previously registered with
> + * gaokun_ec_register_notify().
> + */
> +void gaokun_ec_unregister_notify(struct gaokun_ec *ec, struct notifier_block *nb)
> +{
> +	blocking_notifier_chain_unregister(&ec->notifier_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(gaokun_ec_unregister_notify);
> +
> +/* -------------------------------------------------------------------------- */
> +/* API for PSY */
> +
> +/**
> + * gaokun_ec_psy_multi_read - Read contiguous registers
> + * @ec: The gaokun_ec structure
> + * @reg: The start register
> + * @resp_len: The number of registers to be read
> + * @resp: The buffer to store response sequence
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int gaokun_ec_psy_multi_read(struct gaokun_ec *ec, u8 reg,
> +			     size_t resp_len, u8 *resp)
> +{
> +	u8 ec_req[] = MKREQ(0x02, EC_READ, 1, 0);
> +	u8 ec_resp[] = MKRESP(1);
> +	int i, ret;
> +
> +	for (i = 0; i < resp_len; ++i, reg++) {
> +		refill_req_byte(ec_req, &reg);
> +		ret = gaokun_ec_read(ec, ec_req, sizeof(ec_resp), ec_resp);
> +		if (ret)
> +			return ret;
> +		extr_resp_byte(&resp[i], ec_resp);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(gaokun_ec_psy_multi_read);
> +
> +/* Smart charge */
> +
> +/**
> + * gaokun_ec_psy_get_smart_charge - Get smart charge data from EC
> + * @ec: The gaokun_ec structure
> + * @resp: The buffer to store response sequence (mode, delay, start, end)
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int gaokun_ec_psy_get_smart_charge(struct gaokun_ec *ec,
> +				   u8 resp[GAOKUN_SMART_CHARGE_DATA_SIZE])
> +{
> +	/* GBCM */
> +	u8 ec_req[] = MKREQ(0x02, 0xE4, 0);
> +	u8 ec_resp[] = MKRESP(GAOKUN_SMART_CHARGE_DATA_SIZE);
> +	int ret;
> +
> +	ret = gaokun_ec_read(ec, ec_req, sizeof(ec_resp), ec_resp);
> +	if (ret)
> +		return ret;
> +
> +	extr_resp(resp, ec_resp, GAOKUN_SMART_CHARGE_DATA_SIZE);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(gaokun_ec_psy_get_smart_charge);
> +
> +static inline bool are_thresholds_valid(u8 start, u8 end)
> +{
> +	return end != 0 && start <= end && end <= 100;

Why 100 ? Still feels like an arbitrary number.

Could you add a comment to explain where 100 comes from ?

> +}
> +
> +/**
> + * gaokun_ec_psy_set_smart_charge - Set smart charge data
> + * @ec: The gaokun_ec structure
> + * @req: The sequence to request (mode, delay, start, end)
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int gaokun_ec_psy_set_smart_charge(struct gaokun_ec *ec,
> +				   const u8 req[GAOKUN_SMART_CHARGE_DATA_SIZE])
> +{
> +	/* SBCM */
> +	u8 ec_req[] = MKREQ(0x02, 0xE3, GAOKUN_SMART_CHARGE_DATA_SIZE);
> +
> +	if (!are_thresholds_valid(req[2], req[3]))
> +		return -EINVAL;
> +
> +	refill_req(ec_req, req, GAOKUN_SMART_CHARGE_DATA_SIZE);
> +
> +	return gaokun_ec_write(ec, ec_req);
> +}
> +EXPORT_SYMBOL_GPL(gaokun_ec_psy_set_smart_charge);
> +
> +/* Smart charge enable */
> +
> +/**
> + * gaokun_ec_psy_get_smart_charge_enable - Get smart charge state
> + * @ec: The gaokun_ec structure
> + * @on: The state
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int gaokun_ec_psy_get_smart_charge_enable(struct gaokun_ec *ec, bool *on)
> +{
> +	/* GBAC */
> +	u8 ec_req[] = MKREQ(0x02, 0xE6, 0);

0xE6 == SMART_CHARGE_ENABLE right ?

In which case instead of passing magic numbers inline, it would be nice 
to declare an enum or set of defines that enumerates the command set and 
then pass those values to MKREQ instead of the hex.

Does the first parameter indicate write ? 0x02. 0x03 seems to indicate 
read too ?

If so again please name the hex values as defines/enums and then pass 
the descriptive names.

Looking much more readable now though.

---
bod

Re: [PATCH v4 2/3] platform: arm64: add Huawei Matebook E Go EC driver

[Pengyu Luo]

On Fri, Jan 17, 2025 at 1:31 AM Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
> On 16/01/2025 11:15, Pengyu Luo wrote:
> > There are three variants of which Huawei released the first two
> > simultaneously.
> >
> > Huawei Matebook E Go LTE(sc8180x), codename seems to be gaokun2.
> > Huawei Matebook E Go(sc8280xp@3.0GHz), codename must be gaokun3. (see [1])
> > Huawei Matebook E Go 2023(sc8280xp@2.69GHz), codename should be also gaokun3.
> >
> > Adding support for the latter two variants for now, this driver should
> > also work for the sc8180x variant according to acpi table files, but I
> > don't have the device to test yet.
> >
> > Different from other Qualcomm Snapdragon sc8280xp based machines, the
> > Huawei Matebook E Go uses an embedded controller while others use
> > a system called PMIC GLink. This embedded controller can be used to
> > perform a set of various functions, including, but not limited to:
> >
> > - Battery and charger monitoring;
> > - Charge control and smart charge;
> > - Fn_lock settings;
> > - Tablet lid status;
> > - Temperature sensors;
> > - USB Type-C notifications (ports orientation,  DP alt mode HPD);
> > - USB Type-C PD (according to observation, up to 48w).
> >
> > Add a driver for the EC which creates devices for UCSI and power supply
> > devices.
> >
> > This driver is inspired by the following drivers:
> >          drivers/platform/arm64/acer-aspire1-ec.c
> >          drivers/platform/arm64/lenovo-yoga-c630.c
> >          drivers/platform/x86/huawei-wmi.c
> >
> > [1] https://bugzilla.kernel.org/show_bug.cgi?id=219645
> >
> > Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
> > ---
> >   MAINTAINERS                                   |   7 +
> >   drivers/platform/arm64/Kconfig                |  21 +
> >   drivers/platform/arm64/Makefile               |   1 +
> >   drivers/platform/arm64/huawei-gaokun-ec.c     | 787 ++++++++++++++++++
> >   .../linux/platform_data/huawei-gaokun-ec.h    |  80 ++
> >   5 files changed, 896 insertions(+)
> >   create mode 100644 drivers/platform/arm64/huawei-gaokun-ec.c
> >   create mode 100644 include/linux/platform_data/huawei-gaokun-ec.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 33b9cd11a..27ff24e7d 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -10692,6 +10692,13 @@ S:   Maintained
> >   F:  Documentation/networking/device_drivers/ethernet/huawei/hinic.rst
> >   F:  drivers/net/ethernet/huawei/hinic/
> >
> > +HUAWEI MATEBOOK E GO EMBEDDED CONTROLLER DRIVER
> > +M:   Pengyu Luo <mitltlatltl@gmail.com>
> > +S:   Maintained
> > +F:   Documentation/devicetree/bindings/platform/huawei,gaokun-ec.yaml
> > +F:   drivers/platform/arm64/huawei-gaokun-ec.c
> > +F:   include/linux/platform_data/huawei-gaokun-ec.h
> > +
> >   HUGETLB SUBSYSTEM
> >   M:  Muchun Song <muchun.song@linux.dev>
> >   L:  linux-mm@kvack.org
> > diff --git a/drivers/platform/arm64/Kconfig b/drivers/platform/arm64/Kconfig
> > index f88395ea3..6ceee3c16 100644
> > --- a/drivers/platform/arm64/Kconfig
> > +++ b/drivers/platform/arm64/Kconfig
> > @@ -33,6 +33,27 @@ config EC_ACER_ASPIRE1
> >         laptop where this information is not properly exposed via the
> >         standard ACPI devices.
> >
> > +config EC_HUAWEI_GAOKUN
> > +     tristate "Huawei Matebook E Go Embedded Controller driver"
> > +     depends on ARCH_QCOM || COMPILE_TEST
> > +     depends on I2C
> > +     depends on INPUT
> > +     select AUXILIARY_BUS
> > +
> > +     help
> > +       Say Y here to enable the EC driver for the Huawei Matebook E Go
> > +       which is a sc8280xp-based 2-in-1 tablet. The driver handles battery
> > +       (information, charge control) and USB Type-C DP HPD events as well
> > +       as some misc functions like the lid sensor and temperature sensors,
> > +       etc.
> > +
> > +       This driver provides battery and AC status support for the mentioned
> > +       laptop where this information is not properly exposed via the
> > +       standard ACPI devices.
> > +
> > +       Say M or Y here to include this support.
> > +
> > +
> >   config EC_LENOVO_YOGA_C630
> >       tristate "Lenovo Yoga C630 Embedded Controller driver"
> >       depends on ARCH_QCOM || COMPILE_TEST
> > diff --git a/drivers/platform/arm64/Makefile b/drivers/platform/arm64/Makefile
> > index b2ae9114f..46a99eba3 100644
> > --- a/drivers/platform/arm64/Makefile
> > +++ b/drivers/platform/arm64/Makefile
> > @@ -6,4 +6,5 @@
> >   #
> >
> >   obj-$(CONFIG_EC_ACER_ASPIRE1)       += acer-aspire1-ec.o
> > +obj-$(CONFIG_EC_HUAWEI_GAOKUN)       += huawei-gaokun-ec.o
> >   obj-$(CONFIG_EC_LENOVO_YOGA_C630) += lenovo-yoga-c630.o
> > diff --git a/drivers/platform/arm64/huawei-gaokun-ec.c b/drivers/platform/arm64/huawei-gaokun-ec.c
> > new file mode 100644
> > index 000000000..5b09b7d7c
> > --- /dev/null
> > +++ b/drivers/platform/arm64/huawei-gaokun-ec.c
> > @@ -0,0 +1,787 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * huawei-gaokun-ec - An EC driver for HUAWEI Matebook E Go
> > + *
> > + * Copyright (C) 2024-2025 Pengyu Luo <mitltlatltl@gmail.com>
> > + */
> > +
> > +#include <linux/auxiliary_bus.h>
> > +#include <linux/cleanup.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/hwmon.h>
> > +#include <linux/hwmon-sysfs.h>
> > +#include <linux/i2c.h>
> > +#include <linux/input.h>
> > +#include <linux/notifier.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/platform_data/huawei-gaokun-ec.h>
> > +
> > +#define EC_EVENT             0x06
> > +
> > +/* Also can be found in ACPI specification 12.3 */
> > +#define EC_READ                      0x80
> > +#define EC_WRITE             0x81
> > +#define EC_BURST             0x82
> > +#define EC_QUERY             0x84
> > +
> > +#define EC_FN_LOCK_ON                0x5A
> > +#define EC_FN_LOCK_OFF               0x55
> > +
> > +#define EC_EVENT_LID         0x81
> > +
> > +#define EC_LID_STATE         0x80
> > +#define EC_LID_OPEN          BIT(1)
> > +
> > +#define UCSI_REG_SIZE                7
> > +
> > +/*
> > + * For tx, command sequences are arranged as
> > + * {master_cmd, slave_cmd, data_len, data_seq}
> > + */
> > +#define REQ_HDR_SIZE         3
> > +#define INPUT_SIZE_OFFSET    2
> > +#define REQ_LEN(req) (REQ_HDR_SIZE + req[INPUT_SIZE_OFFSET])
> > +
> > +/*
> > + * For rx, data sequences are arranged as
> > + * {status, data_len(unreliable), data_seq}
> > + */
> > +#define RESP_HDR_SIZE                2
> > +
> > +#define MKREQ(REG0, REG1, SIZE, ...)                 \
> > +{                                                    \
> > +     REG0, REG1, SIZE,                               \
> > +     /* ## will remove comma when SIZE is 0 */       \
> > +     ## __VA_ARGS__,                                 \
> > +     /* make sure len(pkt[3:]) >= SIZE */            \
> > +     [3 + SIZE] = 0,                                 \
> > +}
> > +
> > +#define MKRESP(SIZE)                         \
> > +{                                            \
> > +     [RESP_HDR_SIZE + SIZE - 1] = 0,         \
> > +}
> > +
> > +/* Possible size 1, 4, 20, 24. Most of the time, the size is 1. */
> > +static inline void refill_req(u8 *dest, const u8 *src, size_t size)
> > +{
> > +     memcpy(dest + REQ_HDR_SIZE, src, size);
> > +}
> > +
> > +static inline void refill_req_byte(u8 *dest, const u8 *src)
> > +{
> > +     dest[REQ_HDR_SIZE] = *src;
> > +}
> > +
> > +/* Possible size 1, 2, 4, 7, 20. Most of the time, the size is 1. */
> > +static inline void extr_resp(u8 *dest, const u8 *src, size_t size)
> > +{
> > +     memcpy(dest, src + RESP_HDR_SIZE, size);
> > +}
> > +
> > +static inline void extr_resp_byte(u8 *dest, const u8 *src)
> > +{
> > +     *dest = src[RESP_HDR_SIZE];
> > +}
> > +
> > +static inline void *extr_resp_shallow(const u8 *src)
> > +{
> > +     return src + RESP_HDR_SIZE;
> > +}
> > +
> > +struct gaokun_ec {
> > +     struct i2c_client *client;
> > +     struct mutex lock; /* EC transaction lock */
> > +     struct blocking_notifier_head notifier_list;
> > +     struct device *hwmon_dev;
> > +     struct input_dev *idev;
> > +     bool suspended;
> > +};
> > +
> > +static int gaokun_ec_request(struct gaokun_ec *ec, const u8 *req,
> > +                          size_t resp_len, u8 *resp)
> > +{
> > +     struct i2c_client *client = ec->client;
> > +     struct i2c_msg msgs[2] = {
> > +             {
> > +                     .addr = client->addr,
> > +                     .flags = client->flags,
> > +                     .len = REQ_LEN(req),
> > +                     .buf = req,
> > +             }, {
> > +                     .addr = client->addr,
> > +                     .flags = client->flags | I2C_M_RD,
> > +                     .len = resp_len,
> > +                     .buf = resp,
> > +             },
> > +     };
> > +
> > +     guard(mutex)(&ec->lock);
> > +     i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
>
> You should trap the result code of i2c_transfer() and push it up the
> call stack.
>

This EC uses SMBus Protocol, I guess. Qualcomm I2C driver doesn't support
this though. The response structure define by SMBus I mentioned them above
(Please also check ACPI specification 13.2.5)

+/*
+ * For rx, data sequences are arranged as
+ * {status, data_len(unreliable), data_seq}
+ */

So the first byte is status code.

> > +     usleep_range(2000, 2500); /* have a break, ACPI did this */
> > +
> > +     return *resp ? -EIO : 0;
>
> If the value @ *resp is non-zero return -EIO ?
>
> Why ?
>

Mentioned above.

> > +}
> > +
> > +/* -------------------------------------------------------------------------- */
> > +/* Common API */
> > +
> > +/**
> > + * gaokun_ec_read - Read from EC
> > + * @ec: The gaokun_ec structure
> > + * @req: The sequence to request
> > + * @resp_len: The size to read
> > + * @resp: The buffer to store response sequence
> > + *
> > + * This function is used to read data after writing a magic sequence to EC.
> > + * All EC operations depend on this function.
> > + *
> > + * Huawei uses magic sequences everywhere to complete various functions, all
> > + * these sequences are passed to ECCD(a ACPI method which is quiet similar
> > + * to gaokun_ec_request), there is no good abstraction to generalize these
> > + * sequences, so just wrap it for now. Almost all magic sequences are kept
> > + * in this file.
> > + *
> > + * Return: 0 on success or negative error code.
> > + */
> > +int gaokun_ec_read(struct gaokun_ec *ec, const u8 *req,
> > +                size_t resp_len, u8 *resp)
> > +{
> > +     return gaokun_ec_request(ec, req, resp_len, resp);
> > +}
> > +EXPORT_SYMBOL_GPL(gaokun_ec_read);
> > +
> > +/**
> > + * gaokun_ec_write - Write to EC
> > + * @ec: The gaokun_ec structure
> > + * @req: The sequence to request
> > + *
> > + * This function has no big difference from gaokun_ec_read. When caller care
> > + * only write status and no actual data are returned, then use it.
> > + *
> > + * Return: 0 on success or negative error code.
> > + */
> > +int gaokun_ec_write(struct gaokun_ec *ec, const u8 *req)
> > +{
> > +     u8 ec_resp[] = MKRESP(0);
> > +
> > +     return gaokun_ec_request(ec, req, sizeof(ec_resp), ec_resp);
> > +}
> > +EXPORT_SYMBOL_GPL(gaokun_ec_write);
> > +
> > +int gaokun_ec_read_byte(struct gaokun_ec *ec, const u8 *req, u8 *byte)
> > +{
> > +     int ret;
> > +     u8 ec_resp[] = MKRESP(sizeof(*byte));
> > +
> > +     ret = gaokun_ec_read(ec, req, sizeof(ec_resp), ec_resp);
> > +     extr_resp_byte(byte, ec_resp);
> > +
> > +     return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(gaokun_ec_read_byte);
> > +
> > +/**
> > + * gaokun_ec_register_notify - Register a notifier callback for EC events.
> > + * @ec: The gaokun_ec structure
> > + * @nb: Notifier block pointer to register
> > + *
> > + * Return: 0 on success or negative error code.
> > + */
> > +int gaokun_ec_register_notify(struct gaokun_ec *ec, struct notifier_block *nb)
> > +{
> > +     return blocking_notifier_chain_register(&ec->notifier_list, nb);
> > +}
> > +EXPORT_SYMBOL_GPL(gaokun_ec_register_notify);
> > +
> > +/**
> > + * gaokun_ec_unregister_notify - Unregister notifier callback for EC events.
> > + * @ec: The gaokun_ec structure
> > + * @nb: Notifier block pointer to unregister
> > + *
> > + * Unregister a notifier callback that was previously registered with
> > + * gaokun_ec_register_notify().
> > + */
> > +void gaokun_ec_unregister_notify(struct gaokun_ec *ec, struct notifier_block *nb)
> > +{
> > +     blocking_notifier_chain_unregister(&ec->notifier_list, nb);
> > +}
> > +EXPORT_SYMBOL_GPL(gaokun_ec_unregister_notify);
> > +
> > +/* -------------------------------------------------------------------------- */
> > +/* API for PSY */
> > +
> > +/**
> > + * gaokun_ec_psy_multi_read - Read contiguous registers
> > + * @ec: The gaokun_ec structure
> > + * @reg: The start register
> > + * @resp_len: The number of registers to be read
> > + * @resp: The buffer to store response sequence
> > + *
> > + * Return: 0 on success or negative error code.
> > + */
> > +int gaokun_ec_psy_multi_read(struct gaokun_ec *ec, u8 reg,
> > +                          size_t resp_len, u8 *resp)
> > +{
> > +     u8 ec_req[] = MKREQ(0x02, EC_READ, 1, 0);
> > +     u8 ec_resp[] = MKRESP(1);
> > +     int i, ret;
> > +
> > +     for (i = 0; i < resp_len; ++i, reg++) {
> > +             refill_req_byte(ec_req, &reg);
> > +             ret = gaokun_ec_read(ec, ec_req, sizeof(ec_resp), ec_resp);
> > +             if (ret)
> > +                     return ret;
> > +             extr_resp_byte(&resp[i], ec_resp);
> > +     }
> > +
> > +     return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(gaokun_ec_psy_multi_read);
> > +
> > +/* Smart charge */
> > +
> > +/**
> > + * gaokun_ec_psy_get_smart_charge - Get smart charge data from EC
> > + * @ec: The gaokun_ec structure
> > + * @resp: The buffer to store response sequence (mode, delay, start, end)
> > + *
> > + * Return: 0 on success or negative error code.
> > + */
> > +int gaokun_ec_psy_get_smart_charge(struct gaokun_ec *ec,
> > +                                u8 resp[GAOKUN_SMART_CHARGE_DATA_SIZE])
> > +{
> > +     /* GBCM */
> > +     u8 ec_req[] = MKREQ(0x02, 0xE4, 0);
> > +     u8 ec_resp[] = MKRESP(GAOKUN_SMART_CHARGE_DATA_SIZE);
> > +     int ret;
> > +
> > +     ret = gaokun_ec_read(ec, ec_req, sizeof(ec_resp), ec_resp);
> > +     if (ret)
> > +             return ret;
> > +
> > +     extr_resp(resp, ec_resp, GAOKUN_SMART_CHARGE_DATA_SIZE);
> > +
> > +     return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(gaokun_ec_psy_get_smart_charge);
> > +
> > +static inline bool are_thresholds_valid(u8 start, u8 end)
> > +{
> > +     return end != 0 && start <= end && end <= 100;
>
> Why 100 ? Still feels like an arbitrary number.
>
> Could you add a comment to explain where 100 comes from ?
>

You may don't get it. It is just a battery percentage, greater than 100 is
invalid.

start: The battery percentage at which charging starts (0-100).
stop: The battery percentage at which charging stops (1-100).

 When the laptop is connected to a power adapter, it starts
 charging if the battery level is below the 'start' value. It
 continues charging until the battery reaches the 'stop' level.
 If the battery is already above the 'stop' level, charging is
 paused.

 When the power adapter is always connected, charging will
 begin if the battery level falls below 'start', and charging
 will stop once the battery reaches 'stop'.

> > +}
> > +
> > +/**
> > + * gaokun_ec_psy_set_smart_charge - Set smart charge data
> > + * @ec: The gaokun_ec structure
> > + * @req: The sequence to request (mode, delay, start, end)
> > + *
> > + * Return: 0 on success or negative error code.
> > + */
> > +int gaokun_ec_psy_set_smart_charge(struct gaokun_ec *ec,
> > +                                const u8 req[GAOKUN_SMART_CHARGE_DATA_SIZE])
> > +{
> > +     /* SBCM */
> > +     u8 ec_req[] = MKREQ(0x02, 0xE3, GAOKUN_SMART_CHARGE_DATA_SIZE);
> > +
> > +     if (!are_thresholds_valid(req[2], req[3]))
> > +             return -EINVAL;
> > +
> > +     refill_req(ec_req, req, GAOKUN_SMART_CHARGE_DATA_SIZE);
> > +
> > +     return gaokun_ec_write(ec, ec_req);
> > +}
> > +EXPORT_SYMBOL_GPL(gaokun_ec_psy_set_smart_charge);
> > +
> > +/* Smart charge enable */
> > +
> > +/**
> > + * gaokun_ec_psy_get_smart_charge_enable - Get smart charge state
> > + * @ec: The gaokun_ec structure
> > + * @on: The state
> > + *
> > + * Return: 0 on success or negative error code.
> > + */
> > +int gaokun_ec_psy_get_smart_charge_enable(struct gaokun_ec *ec, bool *on)
> > +{
> > +     /* GBAC */
> > +     u8 ec_req[] = MKREQ(0x02, 0xE6, 0);
>
> 0xE6 == SMART_CHARGE_ENABLE right ?
>
> In which case instead of passing magic numbers inline, it would be nice
> to declare an enum or set of defines that enumerates the command set and
> then pass those values to MKREQ instead of the hex.
>
> Does the first parameter indicate write ? 0x02. 0x03 seems to indicate
> read too ?
>

Sadly, I am not sure. only 0x06 is used when getting event id, so I named
it. Read or write use both 0x02, 0x03.

> If so again please name the hex values as defines/enums and then pass
> the descriptive names.
>

Function names indicate the usage of registers, these registers are used
once only, never reused. If you insist, I will add defines for them.


> Looking much more readable now though.
>
> ---
> bod


Best wishes,
Pengyu

Re: [PATCH v4 2/3] platform: arm64: add Huawei Matebook E Go EC driver

[Maya Matuszczyk]

czw., 16 sty 2025 o 19:17 Pengyu Luo <mitltlatltl@gmail.com> napisał(a):
>
> On Fri, Jan 17, 2025 at 1:31 AM Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
> > On 16/01/2025 11:15, Pengyu Luo wrote:
> > > There are three variants of which Huawei released the first two
> > > simultaneously.
> > >
> > > Huawei Matebook E Go LTE(sc8180x), codename seems to be gaokun2.
> > > Huawei Matebook E Go(sc8280xp@3.0GHz), codename must be gaokun3. (see [1])
> > > Huawei Matebook E Go 2023(sc8280xp@2.69GHz), codename should be also gaokun3.
> > >
> > > Adding support for the latter two variants for now, this driver should
> > > also work for the sc8180x variant according to acpi table files, but I
> > > don't have the device to test yet.
> > >
> > > Different from other Qualcomm Snapdragon sc8280xp based machines, the
> > > Huawei Matebook E Go uses an embedded controller while others use
> > > a system called PMIC GLink. This embedded controller can be used to
> > > perform a set of various functions, including, but not limited to:
> > >
> > > - Battery and charger monitoring;
> > > - Charge control and smart charge;
> > > - Fn_lock settings;
> > > - Tablet lid status;
> > > - Temperature sensors;
> > > - USB Type-C notifications (ports orientation,  DP alt mode HPD);
> > > - USB Type-C PD (according to observation, up to 48w).
> > >
> > > Add a driver for the EC which creates devices for UCSI and power supply
> > > devices.
> > >
> > > This driver is inspired by the following drivers:
> > >          drivers/platform/arm64/acer-aspire1-ec.c
> > >          drivers/platform/arm64/lenovo-yoga-c630.c
> > >          drivers/platform/x86/huawei-wmi.c
> > >
> > > [1] https://bugzilla.kernel.org/show_bug.cgi?id=219645
> > >
> > > Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
> > > ---
> > >   MAINTAINERS                                   |   7 +
> > >   drivers/platform/arm64/Kconfig                |  21 +
> > >   drivers/platform/arm64/Makefile               |   1 +
> > >   drivers/platform/arm64/huawei-gaokun-ec.c     | 787 ++++++++++++++++++
> > >   .../linux/platform_data/huawei-gaokun-ec.h    |  80 ++
> > >   5 files changed, 896 insertions(+)
> > >   create mode 100644 drivers/platform/arm64/huawei-gaokun-ec.c
> > >   create mode 100644 include/linux/platform_data/huawei-gaokun-ec.h
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 33b9cd11a..27ff24e7d 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -10692,6 +10692,13 @@ S:   Maintained
> > >   F:  Documentation/networking/device_drivers/ethernet/huawei/hinic.rst
> > >   F:  drivers/net/ethernet/huawei/hinic/
> > >
> > > +HUAWEI MATEBOOK E GO EMBEDDED CONTROLLER DRIVER
> > > +M:   Pengyu Luo <mitltlatltl@gmail.com>
> > > +S:   Maintained
> > > +F:   Documentation/devicetree/bindings/platform/huawei,gaokun-ec.yaml
> > > +F:   drivers/platform/arm64/huawei-gaokun-ec.c
> > > +F:   include/linux/platform_data/huawei-gaokun-ec.h
> > > +
> > >   HUGETLB SUBSYSTEM
> > >   M:  Muchun Song <muchun.song@linux.dev>
> > >   L:  linux-mm@kvack.org
> > > diff --git a/drivers/platform/arm64/Kconfig b/drivers/platform/arm64/Kconfig
> > > index f88395ea3..6ceee3c16 100644
> > > --- a/drivers/platform/arm64/Kconfig
> > > +++ b/drivers/platform/arm64/Kconfig
> > > @@ -33,6 +33,27 @@ config EC_ACER_ASPIRE1
> > >         laptop where this information is not properly exposed via the
> > >         standard ACPI devices.
> > >
> > > +config EC_HUAWEI_GAOKUN
> > > +     tristate "Huawei Matebook E Go Embedded Controller driver"
> > > +     depends on ARCH_QCOM || COMPILE_TEST
> > > +     depends on I2C
> > > +     depends on INPUT
> > > +     select AUXILIARY_BUS
> > > +
> > > +     help
> > > +       Say Y here to enable the EC driver for the Huawei Matebook E Go
> > > +       which is a sc8280xp-based 2-in-1 tablet. The driver handles battery
> > > +       (information, charge control) and USB Type-C DP HPD events as well
> > > +       as some misc functions like the lid sensor and temperature sensors,
> > > +       etc.
> > > +
> > > +       This driver provides battery and AC status support for the mentioned
> > > +       laptop where this information is not properly exposed via the
> > > +       standard ACPI devices.
> > > +
> > > +       Say M or Y here to include this support.
> > > +
> > > +
> > >   config EC_LENOVO_YOGA_C630
> > >       tristate "Lenovo Yoga C630 Embedded Controller driver"
> > >       depends on ARCH_QCOM || COMPILE_TEST
> > > diff --git a/drivers/platform/arm64/Makefile b/drivers/platform/arm64/Makefile
> > > index b2ae9114f..46a99eba3 100644
> > > --- a/drivers/platform/arm64/Makefile
> > > +++ b/drivers/platform/arm64/Makefile
> > > @@ -6,4 +6,5 @@
> > >   #
> > >
> > >   obj-$(CONFIG_EC_ACER_ASPIRE1)       += acer-aspire1-ec.o
> > > +obj-$(CONFIG_EC_HUAWEI_GAOKUN)       += huawei-gaokun-ec.o
> > >   obj-$(CONFIG_EC_LENOVO_YOGA_C630) += lenovo-yoga-c630.o
> > > diff --git a/drivers/platform/arm64/huawei-gaokun-ec.c b/drivers/platform/arm64/huawei-gaokun-ec.c
> > > new file mode 100644
> > > index 000000000..5b09b7d7c
> > > --- /dev/null
> > > +++ b/drivers/platform/arm64/huawei-gaokun-ec.c
> > > @@ -0,0 +1,787 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * huawei-gaokun-ec - An EC driver for HUAWEI Matebook E Go
> > > + *
> > > + * Copyright (C) 2024-2025 Pengyu Luo <mitltlatltl@gmail.com>
> > > + */
> > > +
> > > +#include <linux/auxiliary_bus.h>
> > > +#include <linux/cleanup.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/device.h>
> > > +#include <linux/hwmon.h>
> > > +#include <linux/hwmon-sysfs.h>
> > > +#include <linux/i2c.h>
> > > +#include <linux/input.h>
> > > +#include <linux/notifier.h>
> > > +#include <linux/module.h>
> > > +#include <linux/mutex.h>
> > > +#include <linux/platform_data/huawei-gaokun-ec.h>
> > > +
> > > +#define EC_EVENT             0x06
> > > +
> > > +/* Also can be found in ACPI specification 12.3 */
> > > +#define EC_READ                      0x80
> > > +#define EC_WRITE             0x81
> > > +#define EC_BURST             0x82
> > > +#define EC_QUERY             0x84
> > > +
> > > +#define EC_FN_LOCK_ON                0x5A
> > > +#define EC_FN_LOCK_OFF               0x55
> > > +
> > > +#define EC_EVENT_LID         0x81
> > > +
> > > +#define EC_LID_STATE         0x80
> > > +#define EC_LID_OPEN          BIT(1)
> > > +
> > > +#define UCSI_REG_SIZE                7
> > > +
> > > +/*
> > > + * For tx, command sequences are arranged as
> > > + * {master_cmd, slave_cmd, data_len, data_seq}
> > > + */
> > > +#define REQ_HDR_SIZE         3
> > > +#define INPUT_SIZE_OFFSET    2
> > > +#define REQ_LEN(req) (REQ_HDR_SIZE + req[INPUT_SIZE_OFFSET])
> > > +
> > > +/*
> > > + * For rx, data sequences are arranged as
> > > + * {status, data_len(unreliable), data_seq}
> > > + */
> > > +#define RESP_HDR_SIZE                2
> > > +
> > > +#define MKREQ(REG0, REG1, SIZE, ...)                 \
> > > +{                                                    \
> > > +     REG0, REG1, SIZE,                               \
> > > +     /* ## will remove comma when SIZE is 0 */       \
> > > +     ## __VA_ARGS__,                                 \
> > > +     /* make sure len(pkt[3:]) >= SIZE */            \
> > > +     [3 + SIZE] = 0,                                 \
> > > +}
> > > +
> > > +#define MKRESP(SIZE)                         \
> > > +{                                            \
> > > +     [RESP_HDR_SIZE + SIZE - 1] = 0,         \
> > > +}
> > > +
> > > +/* Possible size 1, 4, 20, 24. Most of the time, the size is 1. */
> > > +static inline void refill_req(u8 *dest, const u8 *src, size_t size)
> > > +{
> > > +     memcpy(dest + REQ_HDR_SIZE, src, size);
> > > +}
> > > +
> > > +static inline void refill_req_byte(u8 *dest, const u8 *src)
> > > +{
> > > +     dest[REQ_HDR_SIZE] = *src;
> > > +}
> > > +
> > > +/* Possible size 1, 2, 4, 7, 20. Most of the time, the size is 1. */
> > > +static inline void extr_resp(u8 *dest, const u8 *src, size_t size)
> > > +{
> > > +     memcpy(dest, src + RESP_HDR_SIZE, size);
> > > +}
> > > +
> > > +static inline void extr_resp_byte(u8 *dest, const u8 *src)
> > > +{
> > > +     *dest = src[RESP_HDR_SIZE];
> > > +}
> > > +
> > > +static inline void *extr_resp_shallow(const u8 *src)
> > > +{
> > > +     return src + RESP_HDR_SIZE;
> > > +}
> > > +
> > > +struct gaokun_ec {
> > > +     struct i2c_client *client;
> > > +     struct mutex lock; /* EC transaction lock */
> > > +     struct blocking_notifier_head notifier_list;
> > > +     struct device *hwmon_dev;
> > > +     struct input_dev *idev;
> > > +     bool suspended;
> > > +};
> > > +
> > > +static int gaokun_ec_request(struct gaokun_ec *ec, const u8 *req,
> > > +                          size_t resp_len, u8 *resp)
> > > +{
> > > +     struct i2c_client *client = ec->client;
> > > +     struct i2c_msg msgs[2] = {
> > > +             {
> > > +                     .addr = client->addr,
> > > +                     .flags = client->flags,
> > > +                     .len = REQ_LEN(req),
> > > +                     .buf = req,
> > > +             }, {
> > > +                     .addr = client->addr,
> > > +                     .flags = client->flags | I2C_M_RD,
> > > +                     .len = resp_len,
> > > +                     .buf = resp,
> > > +             },
> > > +     };
> > > +
> > > +     guard(mutex)(&ec->lock);
> > > +     i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> >
> > You should trap the result code of i2c_transfer() and push it up the
> > call stack.
> >
>
> This EC uses SMBus Protocol, I guess. Qualcomm I2C driver doesn't support
> this though. The response structure define by SMBus I mentioned them above
> (Please also check ACPI specification 13.2.5)

You can use i2c_smbus_write_byte_data and others with plain i2c controllers,
They all are mentioned and described in Documentation/driver-api/i2c.rst
I've used those APIs(with i2c controller in x1e80100) in my EC driver,
and they worked fine

>
> +/*
> + * For rx, data sequences are arranged as
> + * {status, data_len(unreliable), data_seq}
> + */
>
> So the first byte is status code.
>
> > > +     usleep_range(2000, 2500); /* have a break, ACPI did this */
> > > +
> > > +     return *resp ? -EIO : 0;
> >
> > If the value @ *resp is non-zero return -EIO ?
> >
> > Why ?
> >
>
> Mentioned above.
>
> > > +}
> > > +
> > > +/* -------------------------------------------------------------------------- */
> > > +/* Common API */
> > > +
> > > +/**
> > > + * gaokun_ec_read - Read from EC
> > > + * @ec: The gaokun_ec structure
> > > + * @req: The sequence to request
> > > + * @resp_len: The size to read
> > > + * @resp: The buffer to store response sequence
> > > + *
> > > + * This function is used to read data after writing a magic sequence to EC.
> > > + * All EC operations depend on this function.
> > > + *
> > > + * Huawei uses magic sequences everywhere to complete various functions, all
> > > + * these sequences are passed to ECCD(a ACPI method which is quiet similar
> > > + * to gaokun_ec_request), there is no good abstraction to generalize these
> > > + * sequences, so just wrap it for now. Almost all magic sequences are kept
> > > + * in this file.
> > > + *
> > > + * Return: 0 on success or negative error code.
> > > + */
> > > +int gaokun_ec_read(struct gaokun_ec *ec, const u8 *req,
> > > +                size_t resp_len, u8 *resp)
> > > +{
> > > +     return gaokun_ec_request(ec, req, resp_len, resp);
> > > +}
> > > +EXPORT_SYMBOL_GPL(gaokun_ec_read);
> > > +
> > > +/**
> > > + * gaokun_ec_write - Write to EC
> > > + * @ec: The gaokun_ec structure
> > > + * @req: The sequence to request
> > > + *
> > > + * This function has no big difference from gaokun_ec_read. When caller care
> > > + * only write status and no actual data are returned, then use it.
> > > + *
> > > + * Return: 0 on success or negative error code.
> > > + */
> > > +int gaokun_ec_write(struct gaokun_ec *ec, const u8 *req)
> > > +{
> > > +     u8 ec_resp[] = MKRESP(0);
> > > +
> > > +     return gaokun_ec_request(ec, req, sizeof(ec_resp), ec_resp);
> > > +}
> > > +EXPORT_SYMBOL_GPL(gaokun_ec_write);
> > > +
> > > +int gaokun_ec_read_byte(struct gaokun_ec *ec, const u8 *req, u8 *byte)
> > > +{
> > > +     int ret;
> > > +     u8 ec_resp[] = MKRESP(sizeof(*byte));
> > > +
> > > +     ret = gaokun_ec_read(ec, req, sizeof(ec_resp), ec_resp);
> > > +     extr_resp_byte(byte, ec_resp);
> > > +
> > > +     return ret;
> > > +}
> > > +EXPORT_SYMBOL_GPL(gaokun_ec_read_byte);
> > > +
> > > +/**
> > > + * gaokun_ec_register_notify - Register a notifier callback for EC events.
> > > + * @ec: The gaokun_ec structure
> > > + * @nb: Notifier block pointer to register
> > > + *
> > > + * Return: 0 on success or negative error code.
> > > + */
> > > +int gaokun_ec_register_notify(struct gaokun_ec *ec, struct notifier_block *nb)
> > > +{
> > > +     return blocking_notifier_chain_register(&ec->notifier_list, nb);
> > > +}
> > > +EXPORT_SYMBOL_GPL(gaokun_ec_register_notify);
> > > +
> > > +/**
> > > + * gaokun_ec_unregister_notify - Unregister notifier callback for EC events.
> > > + * @ec: The gaokun_ec structure
> > > + * @nb: Notifier block pointer to unregister
> > > + *
> > > + * Unregister a notifier callback that was previously registered with
> > > + * gaokun_ec_register_notify().
> > > + */
> > > +void gaokun_ec_unregister_notify(struct gaokun_ec *ec, struct notifier_block *nb)
> > > +{
> > > +     blocking_notifier_chain_unregister(&ec->notifier_list, nb);
> > > +}
> > > +EXPORT_SYMBOL_GPL(gaokun_ec_unregister_notify);
> > > +
> > > +/* -------------------------------------------------------------------------- */
> > > +/* API for PSY */
> > > +
> > > +/**
> > > + * gaokun_ec_psy_multi_read - Read contiguous registers
> > > + * @ec: The gaokun_ec structure
> > > + * @reg: The start register
> > > + * @resp_len: The number of registers to be read
> > > + * @resp: The buffer to store response sequence
> > > + *
> > > + * Return: 0 on success or negative error code.
> > > + */
> > > +int gaokun_ec_psy_multi_read(struct gaokun_ec *ec, u8 reg,
> > > +                          size_t resp_len, u8 *resp)
> > > +{
> > > +     u8 ec_req[] = MKREQ(0x02, EC_READ, 1, 0);
> > > +     u8 ec_resp[] = MKRESP(1);
> > > +     int i, ret;
> > > +
> > > +     for (i = 0; i < resp_len; ++i, reg++) {
> > > +             refill_req_byte(ec_req, &reg);
> > > +             ret = gaokun_ec_read(ec, ec_req, sizeof(ec_resp), ec_resp);
> > > +             if (ret)
> > > +                     return ret;
> > > +             extr_resp_byte(&resp[i], ec_resp);
> > > +     }
> > > +
> > > +     return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(gaokun_ec_psy_multi_read);
> > > +
> > > +/* Smart charge */
> > > +
> > > +/**
> > > + * gaokun_ec_psy_get_smart_charge - Get smart charge data from EC
> > > + * @ec: The gaokun_ec structure
> > > + * @resp: The buffer to store response sequence (mode, delay, start, end)
> > > + *
> > > + * Return: 0 on success or negative error code.
> > > + */
> > > +int gaokun_ec_psy_get_smart_charge(struct gaokun_ec *ec,
> > > +                                u8 resp[GAOKUN_SMART_CHARGE_DATA_SIZE])
> > > +{
> > > +     /* GBCM */
> > > +     u8 ec_req[] = MKREQ(0x02, 0xE4, 0);
> > > +     u8 ec_resp[] = MKRESP(GAOKUN_SMART_CHARGE_DATA_SIZE);
> > > +     int ret;
> > > +
> > > +     ret = gaokun_ec_read(ec, ec_req, sizeof(ec_resp), ec_resp);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     extr_resp(resp, ec_resp, GAOKUN_SMART_CHARGE_DATA_SIZE);
> > > +
> > > +     return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(gaokun_ec_psy_get_smart_charge);
> > > +
> > > +static inline bool are_thresholds_valid(u8 start, u8 end)
> > > +{
> > > +     return end != 0 && start <= end && end <= 100;
> >
> > Why 100 ? Still feels like an arbitrary number.
> >
> > Could you add a comment to explain where 100 comes from ?
> >
>
> You may don't get it. It is just a battery percentage, greater than 100 is
> invalid.
>
> start: The battery percentage at which charging starts (0-100).
> stop: The battery percentage at which charging stops (1-100).
>
>  When the laptop is connected to a power adapter, it starts
>  charging if the battery level is below the 'start' value. It
>  continues charging until the battery reaches the 'stop' level.
>  If the battery is already above the 'stop' level, charging is
>  paused.
>
>  When the power adapter is always connected, charging will
>  begin if the battery level falls below 'start', and charging
>  will stop once the battery reaches 'stop'.
>
> > > +}
> > > +
> > > +/**
> > > + * gaokun_ec_psy_set_smart_charge - Set smart charge data
> > > + * @ec: The gaokun_ec structure
> > > + * @req: The sequence to request (mode, delay, start, end)
> > > + *
> > > + * Return: 0 on success or negative error code.
> > > + */
> > > +int gaokun_ec_psy_set_smart_charge(struct gaokun_ec *ec,
> > > +                                const u8 req[GAOKUN_SMART_CHARGE_DATA_SIZE])
> > > +{
> > > +     /* SBCM */
> > > +     u8 ec_req[] = MKREQ(0x02, 0xE3, GAOKUN_SMART_CHARGE_DATA_SIZE);
> > > +
> > > +     if (!are_thresholds_valid(req[2], req[3]))
> > > +             return -EINVAL;
> > > +
> > > +     refill_req(ec_req, req, GAOKUN_SMART_CHARGE_DATA_SIZE);
> > > +
> > > +     return gaokun_ec_write(ec, ec_req);
> > > +}
> > > +EXPORT_SYMBOL_GPL(gaokun_ec_psy_set_smart_charge);
> > > +
> > > +/* Smart charge enable */
> > > +
> > > +/**
> > > + * gaokun_ec_psy_get_smart_charge_enable - Get smart charge state
> > > + * @ec: The gaokun_ec structure
> > > + * @on: The state
> > > + *
> > > + * Return: 0 on success or negative error code.
> > > + */
> > > +int gaokun_ec_psy_get_smart_charge_enable(struct gaokun_ec *ec, bool *on)
> > > +{
> > > +     /* GBAC */
> > > +     u8 ec_req[] = MKREQ(0x02, 0xE6, 0);
> >
> > 0xE6 == SMART_CHARGE_ENABLE right ?
> >
> > In which case instead of passing magic numbers inline, it would be nice
> > to declare an enum or set of defines that enumerates the command set and
> > then pass those values to MKREQ instead of the hex.
> >
> > Does the first parameter indicate write ? 0x02. 0x03 seems to indicate
> > read too ?
> >
>
> Sadly, I am not sure. only 0x06 is used when getting event id, so I named
> it. Read or write use both 0x02, 0x03.
>
> > If so again please name the hex values as defines/enums and then pass
> > the descriptive names.
> >
>
> Function names indicate the usage of registers, these registers are used
> once only, never reused. If you insist, I will add defines for them.
>
>
> > Looking much more readable now though.
> >
> > ---
> > bod
>
>
> Best wishes,
> Pengyu
>

Re: [PATCH v4 2/3] platform: arm64: add Huawei Matebook E Go EC driver

[Pengyu Luo]

On Fri, Jan 17, 2025 at 3:14 AM Maya Matuszczyk <maccraft123mc@gmail.com> wrote:
> czw., 16 sty 2025 o 19:17 Pengyu Luo <mitltlatltl@gmail.com> napisał(a):
> >
> > On Fri, Jan 17, 2025 at 1:31 AM Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
> > > On 16/01/2025 11:15, Pengyu Luo wrote:
> > > > There are three variants of which Huawei released the first two
> > > > simultaneously.
> > > >
> > > > Huawei Matebook E Go LTE(sc8180x), codename seems to be gaokun2.
> > > > Huawei Matebook E Go(sc8280xp@3.0GHz), codename must be gaokun3. (see [1])
> > > > Huawei Matebook E Go 2023(sc8280xp@2.69GHz), codename should be also gaokun3.
> > > >
> > > > Adding support for the latter two variants for now, this driver should
> > > > also work for the sc8180x variant according to acpi table files, but I
> > > > don't have the device to test yet.
> > > >
> > > > Different from other Qualcomm Snapdragon sc8280xp based machines, the
> > > > Huawei Matebook E Go uses an embedded controller while others use
> > > > a system called PMIC GLink. This embedded controller can be used to
> > > > perform a set of various functions, including, but not limited to:
> > > >
> > > > - Battery and charger monitoring;
> > > > - Charge control and smart charge;
> > > > - Fn_lock settings;
> > > > - Tablet lid status;
> > > > - Temperature sensors;
> > > > - USB Type-C notifications (ports orientation,  DP alt mode HPD);
> > > > - USB Type-C PD (according to observation, up to 48w).
> > > >
> > > > Add a driver for the EC which creates devices for UCSI and power supply
> > > > devices.
> > > >
> > > > This driver is inspired by the following drivers:
> > > >          drivers/platform/arm64/acer-aspire1-ec.c
> > > >          drivers/platform/arm64/lenovo-yoga-c630.c
> > > >          drivers/platform/x86/huawei-wmi.c
> > > >
> > > > [1] https://bugzilla.kernel.org/show_bug.cgi?id=219645
> > > >
> > > > Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
> > > > ---
> > > >   MAINTAINERS                                   |   7 +
> > > >   drivers/platform/arm64/Kconfig                |  21 +
> > > >   drivers/platform/arm64/Makefile               |   1 +
> > > >   drivers/platform/arm64/huawei-gaokun-ec.c     | 787 ++++++++++++++++++
> > > >   .../linux/platform_data/huawei-gaokun-ec.h    |  80 ++
> > > >   5 files changed, 896 insertions(+)
> > > >   create mode 100644 drivers/platform/arm64/huawei-gaokun-ec.c
> > > >   create mode 100644 include/linux/platform_data/huawei-gaokun-ec.h
> > > >
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index 33b9cd11a..27ff24e7d 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -10692,6 +10692,13 @@ S:   Maintained
> > > >   F:  Documentation/networking/device_drivers/ethernet/huawei/hinic.rst
> > > >   F:  drivers/net/ethernet/huawei/hinic/
> > > >
> > > > +HUAWEI MATEBOOK E GO EMBEDDED CONTROLLER DRIVER
> > > > +M:   Pengyu Luo <mitltlatltl@gmail.com>
> > > > +S:   Maintained
> > > > +F:   Documentation/devicetree/bindings/platform/huawei,gaokun-ec.yaml
> > > > +F:   drivers/platform/arm64/huawei-gaokun-ec.c
> > > > +F:   include/linux/platform_data/huawei-gaokun-ec.h
> > > > +
> > > >   HUGETLB SUBSYSTEM
> > > >   M:  Muchun Song <muchun.song@linux.dev>
> > > >   L:  linux-mm@kvack.org
> > > > diff --git a/drivers/platform/arm64/Kconfig b/drivers/platform/arm64/Kconfig
> > > > index f88395ea3..6ceee3c16 100644
> > > > --- a/drivers/platform/arm64/Kconfig
> > > > +++ b/drivers/platform/arm64/Kconfig
> > > > @@ -33,6 +33,27 @@ config EC_ACER_ASPIRE1
> > > >         laptop where this information is not properly exposed via the
> > > >         standard ACPI devices.
> > > >
> > > > +config EC_HUAWEI_GAOKUN
> > > > +     tristate "Huawei Matebook E Go Embedded Controller driver"
> > > > +     depends on ARCH_QCOM || COMPILE_TEST
> > > > +     depends on I2C
> > > > +     depends on INPUT
> > > > +     select AUXILIARY_BUS
> > > > +
> > > > +     help
> > > > +       Say Y here to enable the EC driver for the Huawei Matebook E Go
> > > > +       which is a sc8280xp-based 2-in-1 tablet. The driver handles battery
> > > > +       (information, charge control) and USB Type-C DP HPD events as well
> > > > +       as some misc functions like the lid sensor and temperature sensors,
> > > > +       etc.
> > > > +
> > > > +       This driver provides battery and AC status support for the mentioned
> > > > +       laptop where this information is not properly exposed via the
> > > > +       standard ACPI devices.
> > > > +
> > > > +       Say M or Y here to include this support.
> > > > +
> > > > +
> > > >   config EC_LENOVO_YOGA_C630
> > > >       tristate "Lenovo Yoga C630 Embedded Controller driver"
> > > >       depends on ARCH_QCOM || COMPILE_TEST
> > > > diff --git a/drivers/platform/arm64/Makefile b/drivers/platform/arm64/Makefile
> > > > index b2ae9114f..46a99eba3 100644
> > > > --- a/drivers/platform/arm64/Makefile
> > > > +++ b/drivers/platform/arm64/Makefile
> > > > @@ -6,4 +6,5 @@
> > > >   #
> > > >
> > > >   obj-$(CONFIG_EC_ACER_ASPIRE1)       += acer-aspire1-ec.o
> > > > +obj-$(CONFIG_EC_HUAWEI_GAOKUN)       += huawei-gaokun-ec.o
> > > >   obj-$(CONFIG_EC_LENOVO_YOGA_C630) += lenovo-yoga-c630.o
> > > > diff --git a/drivers/platform/arm64/huawei-gaokun-ec.c b/drivers/platform/arm64/huawei-gaokun-ec.c
> > > > new file mode 100644
> > > > index 000000000..5b09b7d7c
> > > > --- /dev/null
> > > > +++ b/drivers/platform/arm64/huawei-gaokun-ec.c
> > > > @@ -0,0 +1,787 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > +/*
> > > > + * huawei-gaokun-ec - An EC driver for HUAWEI Matebook E Go
> > > > + *
> > > > + * Copyright (C) 2024-2025 Pengyu Luo <mitltlatltl@gmail.com>
> > > > + */
> > > > +
> > > > +#include <linux/auxiliary_bus.h>
> > > > +#include <linux/cleanup.h>
> > > > +#include <linux/delay.h>
> > > > +#include <linux/device.h>
> > > > +#include <linux/hwmon.h>
> > > > +#include <linux/hwmon-sysfs.h>
> > > > +#include <linux/i2c.h>
> > > > +#include <linux/input.h>
> > > > +#include <linux/notifier.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/mutex.h>
> > > > +#include <linux/platform_data/huawei-gaokun-ec.h>
> > > > +
> > > > +#define EC_EVENT             0x06
> > > > +
> > > > +/* Also can be found in ACPI specification 12.3 */
> > > > +#define EC_READ                      0x80
> > > > +#define EC_WRITE             0x81
> > > > +#define EC_BURST             0x82
> > > > +#define EC_QUERY             0x84
> > > > +
> > > > +#define EC_FN_LOCK_ON                0x5A
> > > > +#define EC_FN_LOCK_OFF               0x55
> > > > +
> > > > +#define EC_EVENT_LID         0x81
> > > > +
> > > > +#define EC_LID_STATE         0x80
> > > > +#define EC_LID_OPEN          BIT(1)
> > > > +
> > > > +#define UCSI_REG_SIZE                7
> > > > +
> > > > +/*
> > > > + * For tx, command sequences are arranged as
> > > > + * {master_cmd, slave_cmd, data_len, data_seq}
> > > > + */
> > > > +#define REQ_HDR_SIZE         3
> > > > +#define INPUT_SIZE_OFFSET    2
> > > > +#define REQ_LEN(req) (REQ_HDR_SIZE + req[INPUT_SIZE_OFFSET])
> > > > +
> > > > +/*
> > > > + * For rx, data sequences are arranged as
> > > > + * {status, data_len(unreliable), data_seq}
> > > > + */
> > > > +#define RESP_HDR_SIZE                2
> > > > +
> > > > +#define MKREQ(REG0, REG1, SIZE, ...)                 \
> > > > +{                                                    \
> > > > +     REG0, REG1, SIZE,                               \
> > > > +     /* ## will remove comma when SIZE is 0 */       \
> > > > +     ## __VA_ARGS__,                                 \
> > > > +     /* make sure len(pkt[3:]) >= SIZE */            \
> > > > +     [3 + SIZE] = 0,                                 \
> > > > +}
> > > > +
> > > > +#define MKRESP(SIZE)                         \
> > > > +{                                            \
> > > > +     [RESP_HDR_SIZE + SIZE - 1] = 0,         \
> > > > +}
> > > > +
> > > > +/* Possible size 1, 4, 20, 24. Most of the time, the size is 1. */
> > > > +static inline void refill_req(u8 *dest, const u8 *src, size_t size)
> > > > +{
> > > > +     memcpy(dest + REQ_HDR_SIZE, src, size);
> > > > +}
> > > > +
> > > > +static inline void refill_req_byte(u8 *dest, const u8 *src)
> > > > +{
> > > > +     dest[REQ_HDR_SIZE] = *src;
> > > > +}
> > > > +
> > > > +/* Possible size 1, 2, 4, 7, 20. Most of the time, the size is 1. */
> > > > +static inline void extr_resp(u8 *dest, const u8 *src, size_t size)
> > > > +{
> > > > +     memcpy(dest, src + RESP_HDR_SIZE, size);
> > > > +}
> > > > +
> > > > +static inline void extr_resp_byte(u8 *dest, const u8 *src)
> > > > +{
> > > > +     *dest = src[RESP_HDR_SIZE];
> > > > +}
> > > > +
> > > > +static inline void *extr_resp_shallow(const u8 *src)
> > > > +{
> > > > +     return src + RESP_HDR_SIZE;
> > > > +}
> > > > +
> > > > +struct gaokun_ec {
> > > > +     struct i2c_client *client;
> > > > +     struct mutex lock; /* EC transaction lock */
> > > > +     struct blocking_notifier_head notifier_list;
> > > > +     struct device *hwmon_dev;
> > > > +     struct input_dev *idev;
> > > > +     bool suspended;
> > > > +};
> > > > +
> > > > +static int gaokun_ec_request(struct gaokun_ec *ec, const u8 *req,
> > > > +                          size_t resp_len, u8 *resp)
> > > > +{
> > > > +     struct i2c_client *client = ec->client;
> > > > +     struct i2c_msg msgs[2] = {
> > > > +             {
> > > > +                     .addr = client->addr,
> > > > +                     .flags = client->flags,
> > > > +                     .len = REQ_LEN(req),
> > > > +                     .buf = req,
> > > > +             }, {
> > > > +                     .addr = client->addr,
> > > > +                     .flags = client->flags | I2C_M_RD,
> > > > +                     .len = resp_len,
> > > > +                     .buf = resp,
> > > > +             },
> > > > +     };
> > > > +
> > > > +     guard(mutex)(&ec->lock);
> > > > +     i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> > >
> > > You should trap the result code of i2c_transfer() and push it up the
> > > call stack.
> > >
> >
> > This EC uses SMBus Protocol, I guess. Qualcomm I2C driver doesn't support
> > this though. The response structure define by SMBus I mentioned them above
> > (Please also check ACPI specification 13.2.5)
>
> You can use i2c_smbus_write_byte_data and others with plain i2c controllers,
> They all are mentioned and described in Documentation/driver-api/i2c.rst
> I've used those APIs(with i2c controller in x1e80100) in my EC driver,
> and they worked fine
>

I talked about this in detail. See
Link: https://lore.kernel.org/linux-arm-msm/20250103071957.7902-1-mitltlatltl@gmail.com

> >
> > +/*
> > + * For rx, data sequences are arranged as
> > + * {status, data_len(unreliable), data_seq}
> > + */
> >
> > So the first byte is status code.
> >
> > > > +     usleep_range(2000, 2500); /* have a break, ACPI did this */
> > > > +
> > > > +     return *resp ? -EIO : 0;
> > >
> > > If the value @ *resp is non-zero return -EIO ?
> > >
> > > Why ?
> > >
> >
> > Mentioned above.
> >
> > > > +}
> > > > +
> > > > +/* -------------------------------------------------------------------------- */
> > > > +/* Common API */
> > > > +
> > > > +/**
> > > > + * gaokun_ec_read - Read from EC
> > > > + * @ec: The gaokun_ec structure
> > > > + * @req: The sequence to request
> > > > + * @resp_len: The size to read
> > > > + * @resp: The buffer to store response sequence
> > > > + *
> > > > + * This function is used to read data after writing a magic sequence to EC.
> > > > + * All EC operations depend on this function.
> > > > + *
> > > > + * Huawei uses magic sequences everywhere to complete various functions, all
> > > > + * these sequences are passed to ECCD(a ACPI method which is quiet similar
> > > > + * to gaokun_ec_request), there is no good abstraction to generalize these
> > > > + * sequences, so just wrap it for now. Almost all magic sequences are kept
> > > > + * in this file.
> > > > + *
> > > > + * Return: 0 on success or negative error code.
> > > > + */
> > > > +int gaokun_ec_read(struct gaokun_ec *ec, const u8 *req,
> > > > +                size_t resp_len, u8 *resp)
> > > > +{
> > > > +     return gaokun_ec_request(ec, req, resp_len, resp);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(gaokun_ec_read);
> > > > +
> > > > +/**
> > > > + * gaokun_ec_write - Write to EC
> > > > + * @ec: The gaokun_ec structure
> > > > + * @req: The sequence to request
> > > > + *
> > > > + * This function has no big difference from gaokun_ec_read. When caller care
> > > > + * only write status and no actual data are returned, then use it.
> > > > + *
> > > > + * Return: 0 on success or negative error code.
> > > > + */
> > > > +int gaokun_ec_write(struct gaokun_ec *ec, const u8 *req)
> > > > +{
> > > > +     u8 ec_resp[] = MKRESP(0);
> > > > +
> > > > +     return gaokun_ec_request(ec, req, sizeof(ec_resp), ec_resp);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(gaokun_ec_write);
> > > > +
> > > > +int gaokun_ec_read_byte(struct gaokun_ec *ec, const u8 *req, u8 *byte)
> > > > +{
> > > > +     int ret;
> > > > +     u8 ec_resp[] = MKRESP(sizeof(*byte));
> > > > +
> > > > +     ret = gaokun_ec_read(ec, req, sizeof(ec_resp), ec_resp);
> > > > +     extr_resp_byte(byte, ec_resp);
> > > > +
> > > > +     return ret;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(gaokun_ec_read_byte);
> > > > +
> > > > +/**
> > > > + * gaokun_ec_register_notify - Register a notifier callback for EC events.
> > > > + * @ec: The gaokun_ec structure
> > > > + * @nb: Notifier block pointer to register
> > > > + *
> > > > + * Return: 0 on success or negative error code.
> > > > + */
> > > > +int gaokun_ec_register_notify(struct gaokun_ec *ec, struct notifier_block *nb)
> > > > +{
> > > > +     return blocking_notifier_chain_register(&ec->notifier_list, nb);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(gaokun_ec_register_notify);
> > > > +
> > > > +/**
> > > > + * gaokun_ec_unregister_notify - Unregister notifier callback for EC events.
> > > > + * @ec: The gaokun_ec structure
> > > > + * @nb: Notifier block pointer to unregister
> > > > + *
> > > > + * Unregister a notifier callback that was previously registered with
> > > > + * gaokun_ec_register_notify().
> > > > + */
> > > > +void gaokun_ec_unregister_notify(struct gaokun_ec *ec, struct notifier_block *nb)
> > > > +{
> > > > +     blocking_notifier_chain_unregister(&ec->notifier_list, nb);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(gaokun_ec_unregister_notify);
> > > > +
> > > > +/* -------------------------------------------------------------------------- */
> > > > +/* API for PSY */
> > > > +
> > > > +/**
> > > > + * gaokun_ec_psy_multi_read - Read contiguous registers
> > > > + * @ec: The gaokun_ec structure
> > > > + * @reg: The start register
> > > > + * @resp_len: The number of registers to be read
> > > > + * @resp: The buffer to store response sequence
> > > > + *
> > > > + * Return: 0 on success or negative error code.
> > > > + */
> > > > +int gaokun_ec_psy_multi_read(struct gaokun_ec *ec, u8 reg,
> > > > +                          size_t resp_len, u8 *resp)
> > > > +{
> > > > +     u8 ec_req[] = MKREQ(0x02, EC_READ, 1, 0);
> > > > +     u8 ec_resp[] = MKRESP(1);
> > > > +     int i, ret;
> > > > +
> > > > +     for (i = 0; i < resp_len; ++i, reg++) {
> > > > +             refill_req_byte(ec_req, &reg);
> > > > +             ret = gaokun_ec_read(ec, ec_req, sizeof(ec_resp), ec_resp);
> > > > +             if (ret)
> > > > +                     return ret;
> > > > +             extr_resp_byte(&resp[i], ec_resp);
> > > > +     }
> > > > +
> > > > +     return 0;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(gaokun_ec_psy_multi_read);
> > > > +
> > > > +/* Smart charge */
> > > > +
> > > > +/**
> > > > + * gaokun_ec_psy_get_smart_charge - Get smart charge data from EC
> > > > + * @ec: The gaokun_ec structure
> > > > + * @resp: The buffer to store response sequence (mode, delay, start, end)
> > > > + *
> > > > + * Return: 0 on success or negative error code.
> > > > + */
> > > > +int gaokun_ec_psy_get_smart_charge(struct gaokun_ec *ec,
> > > > +                                u8 resp[GAOKUN_SMART_CHARGE_DATA_SIZE])
> > > > +{
> > > > +     /* GBCM */
> > > > +     u8 ec_req[] = MKREQ(0x02, 0xE4, 0);
> > > > +     u8 ec_resp[] = MKRESP(GAOKUN_SMART_CHARGE_DATA_SIZE);
> > > > +     int ret;
> > > > +
> > > > +     ret = gaokun_ec_read(ec, ec_req, sizeof(ec_resp), ec_resp);
> > > > +     if (ret)
> > > > +             return ret;
> > > > +
> > > > +     extr_resp(resp, ec_resp, GAOKUN_SMART_CHARGE_DATA_SIZE);
> > > > +
> > > > +     return 0;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(gaokun_ec_psy_get_smart_charge);
> > > > +
> > > > +static inline bool are_thresholds_valid(u8 start, u8 end)
> > > > +{
> > > > +     return end != 0 && start <= end && end <= 100;
> > >
> > > Why 100 ? Still feels like an arbitrary number.
> > >
> > > Could you add a comment to explain where 100 comes from ?
> > >
> >
> > You may don't get it. It is just a battery percentage, greater than 100 is
> > invalid.
> >
> > start: The battery percentage at which charging starts (0-100).
> > stop: The battery percentage at which charging stops (1-100).
> >
> >  When the laptop is connected to a power adapter, it starts
> >  charging if the battery level is below the 'start' value. It
> >  continues charging until the battery reaches the 'stop' level.
> >  If the battery is already above the 'stop' level, charging is
> >  paused.
> >
> >  When the power adapter is always connected, charging will
> >  begin if the battery level falls below 'start', and charging
> >  will stop once the battery reaches 'stop'.
> >
> > > > +}
> > > > +
> > > > +/**
> > > > + * gaokun_ec_psy_set_smart_charge - Set smart charge data
> > > > + * @ec: The gaokun_ec structure
> > > > + * @req: The sequence to request (mode, delay, start, end)
> > > > + *
> > > > + * Return: 0 on success or negative error code.
> > > > + */
> > > > +int gaokun_ec_psy_set_smart_charge(struct gaokun_ec *ec,
> > > > +                                const u8 req[GAOKUN_SMART_CHARGE_DATA_SIZE])
> > > > +{
> > > > +     /* SBCM */
> > > > +     u8 ec_req[] = MKREQ(0x02, 0xE3, GAOKUN_SMART_CHARGE_DATA_SIZE);
> > > > +
> > > > +     if (!are_thresholds_valid(req[2], req[3]))
> > > > +             return -EINVAL;
> > > > +
> > > > +     refill_req(ec_req, req, GAOKUN_SMART_CHARGE_DATA_SIZE);
> > > > +
> > > > +     return gaokun_ec_write(ec, ec_req);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(gaokun_ec_psy_set_smart_charge);
> > > > +
> > > > +/* Smart charge enable */
> > > > +
> > > > +/**
> > > > + * gaokun_ec_psy_get_smart_charge_enable - Get smart charge state
> > > > + * @ec: The gaokun_ec structure
> > > > + * @on: The state
> > > > + *
> > > > + * Return: 0 on success or negative error code.
> > > > + */
> > > > +int gaokun_ec_psy_get_smart_charge_enable(struct gaokun_ec *ec, bool *on)
> > > > +{
> > > > +     /* GBAC */
> > > > +     u8 ec_req[] = MKREQ(0x02, 0xE6, 0);
> > >
> > > 0xE6 == SMART_CHARGE_ENABLE right ?
> > >
> > > In which case instead of passing magic numbers inline, it would be nice
> > > to declare an enum or set of defines that enumerates the command set and
> > > then pass those values to MKREQ instead of the hex.
> > >
> > > Does the first parameter indicate write ? 0x02. 0x03 seems to indicate
> > > read too ?
> > >
> >
> > Sadly, I am not sure. only 0x06 is used when getting event id, so I named
> > it. Read or write use both 0x02, 0x03.
> >
> > > If so again please name the hex values as defines/enums and then pass
> > > the descriptive names.
> > >
> >
> > Function names indicate the usage of registers, these registers are used
> > once only, never reused. If you insist, I will add defines for them.
> >
> >
> > > Looking much more readable now though.
> > >
> > > ---
> > > bod
> >
> >
> > Best wishes,
> > Pengyu
> >

Best wishes,
Pengyu

Re: [PATCH v4 2/3] platform: arm64: add Huawei Matebook E Go EC driver

[Bryan O'Donoghue]

On 16/01/2025 18:15, Pengyu Luo wrote:
> On Fri, Jan 17, 2025 at 1:31 AM Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
>> On 16/01/2025 11:15, Pengyu Luo wrote:
>>> +
>>> +     guard(mutex)(&ec->lock);
>>> +     i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
>>
>> You should trap the result code of i2c_transfer() and push it up the
>> call stack.
>>
> 
> This EC uses SMBus Protocol, I guess. Qualcomm I2C driver doesn't support
> this though. The response structure define by SMBus I mentioned them above
> (Please also check ACPI specification 13.2.5)

What difference does that make ? The i2c controller itself can return 
error codes via i2c_transfer().

You should trap those error codes and take action if they happen.

> 
> +/*
> + * For rx, data sequences are arranged as
> + * {status, data_len(unreliable), data_seq}
> + */
> 
> So the first byte is status code.
> 
>>> +     usleep_range(2000, 2500); /* have a break, ACPI did this */
>>> +
>>> +     return *resp ? -EIO : 0;
>>
>> If the value @ *resp is non-zero return -EIO ?
>>
>> Why ?
>>
> 
> Mentioned above.

Right, please try to take the result code of i2c_transfer() and if it 
indicates error, transmit that error up the call stack.


> 
>>> +}
>>> +
>>> +/* -------------------------------------------------------------------------- */
>>> +/* Common API */
>>> +
>>> +/**
>>> + * gaokun_ec_read - Read from EC
>>> + * @ec: The gaokun_ec structure
>>> + * @req: The sequence to request
>>> + * @resp_len: The size to read
>>> + * @resp: The buffer to store response sequence
>>> + *
>>> + * This function is used to read data after writing a magic sequence to EC.
>>> + * All EC operations depend on this function.
>>> + *
>>> + * Huawei uses magic sequences everywhere to complete various functions, all
>>> + * these sequences are passed to ECCD(a ACPI method which is quiet similar
>>> + * to gaokun_ec_request), there is no good abstraction to generalize these
>>> + * sequences, so just wrap it for now. Almost all magic sequences are kept
>>> + * in this file.
>>> + *
>>> + * Return: 0 on success or negative error code.
>>> + */
>>> +int gaokun_ec_read(struct gaokun_ec *ec, const u8 *req,
>>> +                size_t resp_len, u8 *resp)
>>> +{
>>> +     return gaokun_ec_request(ec, req, resp_len, resp);
>>> +}
>>> +EXPORT_SYMBOL_GPL(gaokun_ec_read);
>>> +
>>> +/**
>>> + * gaokun_ec_write - Write to EC
>>> + * @ec: The gaokun_ec structure
>>> + * @req: The sequence to request
>>> + *
>>> + * This function has no big difference from gaokun_ec_read. When caller care
>>> + * only write status and no actual data are returned, then use it.
>>> + *
>>> + * Return: 0 on success or negative error code.
>>> + */
>>> +int gaokun_ec_write(struct gaokun_ec *ec, const u8 *req)
>>> +{
>>> +     u8 ec_resp[] = MKRESP(0);
>>> +
>>> +     return gaokun_ec_request(ec, req, sizeof(ec_resp), ec_resp);
>>> +}
>>> +EXPORT_SYMBOL_GPL(gaokun_ec_write);
>>> +
>>> +int gaokun_ec_read_byte(struct gaokun_ec *ec, const u8 *req, u8 *byte)
>>> +{
>>> +     int ret;
>>> +     u8 ec_resp[] = MKRESP(sizeof(*byte));
>>> +
>>> +     ret = gaokun_ec_read(ec, req, sizeof(ec_resp), ec_resp);
>>> +     extr_resp_byte(byte, ec_resp);
>>> +
>>> +     return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(gaokun_ec_read_byte);
>>> +
>>> +/**
>>> + * gaokun_ec_register_notify - Register a notifier callback for EC events.
>>> + * @ec: The gaokun_ec structure
>>> + * @nb: Notifier block pointer to register
>>> + *
>>> + * Return: 0 on success or negative error code.
>>> + */
>>> +int gaokun_ec_register_notify(struct gaokun_ec *ec, struct notifier_block *nb)
>>> +{
>>> +     return blocking_notifier_chain_register(&ec->notifier_list, nb);
>>> +}
>>> +EXPORT_SYMBOL_GPL(gaokun_ec_register_notify);
>>> +
>>> +/**
>>> + * gaokun_ec_unregister_notify - Unregister notifier callback for EC events.
>>> + * @ec: The gaokun_ec structure
>>> + * @nb: Notifier block pointer to unregister
>>> + *
>>> + * Unregister a notifier callback that was previously registered with
>>> + * gaokun_ec_register_notify().
>>> + */
>>> +void gaokun_ec_unregister_notify(struct gaokun_ec *ec, struct notifier_block *nb)
>>> +{
>>> +     blocking_notifier_chain_unregister(&ec->notifier_list, nb);
>>> +}
>>> +EXPORT_SYMBOL_GPL(gaokun_ec_unregister_notify);
>>> +
>>> +/* -------------------------------------------------------------------------- */
>>> +/* API for PSY */
>>> +
>>> +/**
>>> + * gaokun_ec_psy_multi_read - Read contiguous registers
>>> + * @ec: The gaokun_ec structure
>>> + * @reg: The start register
>>> + * @resp_len: The number of registers to be read
>>> + * @resp: The buffer to store response sequence
>>> + *
>>> + * Return: 0 on success or negative error code.
>>> + */
>>> +int gaokun_ec_psy_multi_read(struct gaokun_ec *ec, u8 reg,
>>> +                          size_t resp_len, u8 *resp)
>>> +{
>>> +     u8 ec_req[] = MKREQ(0x02, EC_READ, 1, 0);
>>> +     u8 ec_resp[] = MKRESP(1);
>>> +     int i, ret;
>>> +
>>> +     for (i = 0; i < resp_len; ++i, reg++) {
>>> +             refill_req_byte(ec_req, &reg);
>>> +             ret = gaokun_ec_read(ec, ec_req, sizeof(ec_resp), ec_resp);
>>> +             if (ret)
>>> +                     return ret;
>>> +             extr_resp_byte(&resp[i], ec_resp);
>>> +     }
>>> +
>>> +     return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(gaokun_ec_psy_multi_read);
>>> +
>>> +/* Smart charge */
>>> +
>>> +/**
>>> + * gaokun_ec_psy_get_smart_charge - Get smart charge data from EC
>>> + * @ec: The gaokun_ec structure
>>> + * @resp: The buffer to store response sequence (mode, delay, start, end)
>>> + *
>>> + * Return: 0 on success or negative error code.
>>> + */
>>> +int gaokun_ec_psy_get_smart_charge(struct gaokun_ec *ec,
>>> +                                u8 resp[GAOKUN_SMART_CHARGE_DATA_SIZE])
>>> +{
>>> +     /* GBCM */
>>> +     u8 ec_req[] = MKREQ(0x02, 0xE4, 0);
>>> +     u8 ec_resp[] = MKRESP(GAOKUN_SMART_CHARGE_DATA_SIZE);
>>> +     int ret;
>>> +
>>> +     ret = gaokun_ec_read(ec, ec_req, sizeof(ec_resp), ec_resp);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     extr_resp(resp, ec_resp, GAOKUN_SMART_CHARGE_DATA_SIZE);
>>> +
>>> +     return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(gaokun_ec_psy_get_smart_charge);
>>> +
>>> +static inline bool are_thresholds_valid(u8 start, u8 end)
>>> +{
>>> +     return end != 0 && start <= end && end <= 100;
>>
>> Why 100 ? Still feels like an arbitrary number.
>>
>> Could you add a comment to explain where 100 comes from ?
>>
> 
> You may don't get it. It is just a battery percentage, greater than 100 is
> invalid.

100 meaning maximum capacity, good stuff.

Please use a define with a descriptive name. That way the meaning is 
obvious.

In fact if the name of the function related to battery capacity then the 
meaning of the numbers would be more obvious.

static inline bool validate_battery_threshold_range(u8 start, u8 end) {
	return end != 0 && start <= end && end <= 100;
}

> 
> start: The battery percentage at which charging starts (0-100).
> stop: The battery percentage at which charging stops (1-100).

Or just add this comment directly above the function.

---
bod

Re: [PATCH v4 2/3] platform: arm64: add Huawei Matebook E Go EC driver

[kernel test robot]

Hi Pengyu,

kernel test robot noticed the following build warnings:

[auto build test WARNING on b323d8e7bc03d27dec646bfdccb7d1a92411f189]

url:    https://github.com/intel-lab-lkp/linux/commits/Pengyu-Luo/dt-bindings-platform-Add-Huawei-Matebook-E-Go-EC/20250116-192206
base:   b323d8e7bc03d27dec646bfdccb7d1a92411f189
patch link:    https://lore.kernel.org/r/20250116111559.83641-3-mitltlatltl%40gmail.com
patch subject: [PATCH v4 2/3] platform: arm64: add Huawei Matebook E Go EC driver
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20250117/202501171826.NGZwFrgW-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250117/202501171826.NGZwFrgW-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501171826.NGZwFrgW-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/platform/arm64/huawei-gaokun-ec.c: In function 'extr_resp_shallow':
>> drivers/platform/arm64/huawei-gaokun-ec.c:91:20: warning: return discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
      91 |         return src + RESP_HDR_SIZE;
         |                    ^
   drivers/platform/arm64/huawei-gaokun-ec.c: In function 'gaokun_ec_request':
>> drivers/platform/arm64/huawei-gaokun-ec.c:112:32: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     112 |                         .buf = req,
         |                                ^~~


vim +/const +91 drivers/platform/arm64/huawei-gaokun-ec.c

    88	
    89	static inline void *extr_resp_shallow(const u8 *src)
    90	{
  > 91		return src + RESP_HDR_SIZE;
    92	}
    93	
    94	struct gaokun_ec {
    95		struct i2c_client *client;
    96		struct mutex lock; /* EC transaction lock */
    97		struct blocking_notifier_head notifier_list;
    98		struct device *hwmon_dev;
    99		struct input_dev *idev;
   100		bool suspended;
   101	};
   102	
   103	static int gaokun_ec_request(struct gaokun_ec *ec, const u8 *req,
   104				     size_t resp_len, u8 *resp)
   105	{
   106		struct i2c_client *client = ec->client;
   107		struct i2c_msg msgs[2] = {
   108			{
   109				.addr = client->addr,
   110				.flags = client->flags,
   111				.len = REQ_LEN(req),
 > 112				.buf = req,
   113			}, {
   114				.addr = client->addr,
   115				.flags = client->flags | I2C_M_RD,
   116				.len = resp_len,
   117				.buf = resp,
   118			},
   119		};
   120	
   121		guard(mutex)(&ec->lock);
   122		i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
   123		usleep_range(2000, 2500); /* have a break, ACPI did this */
   124	
   125		return *resp ? -EIO : 0;
   126	}
   127	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 2/3] platform: arm64: add Huawei Matebook E Go EC driver

[kernel test robot]

Hi Pengyu,

kernel test robot noticed the following build errors:

[auto build test ERROR on b323d8e7bc03d27dec646bfdccb7d1a92411f189]

url:    https://github.com/intel-lab-lkp/linux/commits/Pengyu-Luo/dt-bindings-platform-Add-Huawei-Matebook-E-Go-EC/20250116-192206
base:   b323d8e7bc03d27dec646bfdccb7d1a92411f189
patch link:    https://lore.kernel.org/r/20250116111559.83641-3-mitltlatltl%40gmail.com
patch subject: [PATCH v4 2/3] platform: arm64: add Huawei Matebook E Go EC driver
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20250117/202501172102.HQ2qqllb-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250117/202501172102.HQ2qqllb-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501172102.HQ2qqllb-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/platform/arm64/huawei-gaokun-ec.c:8:
   In file included from include/linux/auxiliary_bus.h:11:
   In file included from include/linux/device.h:32:
   In file included from include/linux/device/driver.h:21:
   In file included from include/linux/module.h:19:
   In file included from include/linux/elf.h:6:
   In file included from arch/s390/include/asm/elf.h:181:
   In file included from arch/s390/include/asm/mmu_context.h:11:
   In file included from arch/s390/include/asm/pgalloc.h:18:
   In file included from include/linux/mm.h:2224:
   include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     505 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     512 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     525 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
>> drivers/platform/arm64/huawei-gaokun-ec.c:91:9: error: returning 'const u8 *' (aka 'const unsigned char *') from a function with result type 'void *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
      91 |         return src + RESP_HDR_SIZE;
         |                ^~~~~~~~~~~~~~~~~~~
>> drivers/platform/arm64/huawei-gaokun-ec.c:112:11: error: initializing '__u8 *' (aka 'unsigned char *') with an expression of type 'const u8 *' (aka 'const unsigned char *') discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
     112 |                         .buf = req,
         |                                ^~~
   3 warnings and 2 errors generated.


vim +91 drivers/platform/arm64/huawei-gaokun-ec.c

    88	
    89	static inline void *extr_resp_shallow(const u8 *src)
    90	{
  > 91		return src + RESP_HDR_SIZE;
    92	}
    93	
    94	struct gaokun_ec {
    95		struct i2c_client *client;
    96		struct mutex lock; /* EC transaction lock */
    97		struct blocking_notifier_head notifier_list;
    98		struct device *hwmon_dev;
    99		struct input_dev *idev;
   100		bool suspended;
   101	};
   102	
   103	static int gaokun_ec_request(struct gaokun_ec *ec, const u8 *req,
   104				     size_t resp_len, u8 *resp)
   105	{
   106		struct i2c_client *client = ec->client;
   107		struct i2c_msg msgs[2] = {
   108			{
   109				.addr = client->addr,
   110				.flags = client->flags,
   111				.len = REQ_LEN(req),
 > 112				.buf = req,
   113			}, {
   114				.addr = client->addr,
   115				.flags = client->flags | I2C_M_RD,
   116				.len = resp_len,
   117				.buf = resp,
   118			},
   119		};
   120	
   121		guard(mutex)(&ec->lock);
   122		i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
   123		usleep_range(2000, 2500); /* have a break, ACPI did this */
   124	
   125		return *resp ? -EIO : 0;
   126	}
   127	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 2/3] platform: arm64: add Huawei Matebook E Go EC driver

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 8708 bytes --]

On Fri, 17 Jan 2025, Bryan O'Donoghue wrote:

> On 16/01/2025 18:15, Pengyu Luo wrote:
> > On Fri, Jan 17, 2025 at 1:31 AM Bryan O'Donoghue
> > <bryan.odonoghue@linaro.org> wrote:
> > > On 16/01/2025 11:15, Pengyu Luo wrote:
> > > > +
> > > > +     guard(mutex)(&ec->lock);
> > > > +     i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> > > 
> > > You should trap the result code of i2c_transfer() and push it up the
> > > call stack.
> > > 
> > 
> > This EC uses SMBus Protocol, I guess. Qualcomm I2C driver doesn't support
> > this though. The response structure define by SMBus I mentioned them above
> > (Please also check ACPI specification 13.2.5)
> 
> What difference does that make ? The i2c controller itself can return error
> codes via i2c_transfer().
> 
> You should trap those error codes and take action if they happen.
> 
> > 
> > +/*
> > + * For rx, data sequences are arranged as
> > + * {status, data_len(unreliable), data_seq}
> > + */
> > 
> > So the first byte is status code.
> > 
> > > > +     usleep_range(2000, 2500); /* have a break, ACPI did this */
> > > > +
> > > > +     return *resp ? -EIO : 0;
> > > 
> > > If the value @ *resp is non-zero return -EIO ?
> > > 
> > > Why ?
> > > 
> > 
> > Mentioned above.
> 
> Right, please try to take the result code of i2c_transfer() and if it
> indicates error, transmit that error up the call stack.
> 
> 
> > 
> > > > +}
> > > > +
> > > > +/*
> > > > --------------------------------------------------------------------------
> > > > */
> > > > +/* Common API */
> > > > +
> > > > +/**
> > > > + * gaokun_ec_read - Read from EC
> > > > + * @ec: The gaokun_ec structure
> > > > + * @req: The sequence to request
> > > > + * @resp_len: The size to read
> > > > + * @resp: The buffer to store response sequence
> > > > + *
> > > > + * This function is used to read data after writing a magic sequence to
> > > > EC.
> > > > + * All EC operations depend on this function.
> > > > + *
> > > > + * Huawei uses magic sequences everywhere to complete various
> > > > functions, all
> > > > + * these sequences are passed to ECCD(a ACPI method which is quiet
> > > > similar
> > > > + * to gaokun_ec_request), there is no good abstraction to generalize
> > > > these
> > > > + * sequences, so just wrap it for now. Almost all magic sequences are
> > > > kept
> > > > + * in this file.
> > > > + *
> > > > + * Return: 0 on success or negative error code.
> > > > + */
> > > > +int gaokun_ec_read(struct gaokun_ec *ec, const u8 *req,
> > > > +                size_t resp_len, u8 *resp)
> > > > +{
> > > > +     return gaokun_ec_request(ec, req, resp_len, resp);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(gaokun_ec_read);
> > > > +
> > > > +/**
> > > > + * gaokun_ec_write - Write to EC
> > > > + * @ec: The gaokun_ec structure
> > > > + * @req: The sequence to request
> > > > + *
> > > > + * This function has no big difference from gaokun_ec_read. When caller
> > > > care
> > > > + * only write status and no actual data are returned, then use it.
> > > > + *
> > > > + * Return: 0 on success or negative error code.
> > > > + */
> > > > +int gaokun_ec_write(struct gaokun_ec *ec, const u8 *req)
> > > > +{
> > > > +     u8 ec_resp[] = MKRESP(0);
> > > > +
> > > > +     return gaokun_ec_request(ec, req, sizeof(ec_resp), ec_resp);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(gaokun_ec_write);
> > > > +
> > > > +int gaokun_ec_read_byte(struct gaokun_ec *ec, const u8 *req, u8 *byte)
> > > > +{
> > > > +     int ret;
> > > > +     u8 ec_resp[] = MKRESP(sizeof(*byte));
> > > > +
> > > > +     ret = gaokun_ec_read(ec, req, sizeof(ec_resp), ec_resp);
> > > > +     extr_resp_byte(byte, ec_resp);
> > > > +
> > > > +     return ret;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(gaokun_ec_read_byte);
> > > > +
> > > > +/**
> > > > + * gaokun_ec_register_notify - Register a notifier callback for EC
> > > > events.
> > > > + * @ec: The gaokun_ec structure
> > > > + * @nb: Notifier block pointer to register
> > > > + *
> > > > + * Return: 0 on success or negative error code.
> > > > + */
> > > > +int gaokun_ec_register_notify(struct gaokun_ec *ec, struct
> > > > notifier_block *nb)
> > > > +{
> > > > +     return blocking_notifier_chain_register(&ec->notifier_list, nb);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(gaokun_ec_register_notify);
> > > > +
> > > > +/**
> > > > + * gaokun_ec_unregister_notify - Unregister notifier callback for EC
> > > > events.
> > > > + * @ec: The gaokun_ec structure
> > > > + * @nb: Notifier block pointer to unregister
> > > > + *
> > > > + * Unregister a notifier callback that was previously registered with
> > > > + * gaokun_ec_register_notify().
> > > > + */
> > > > +void gaokun_ec_unregister_notify(struct gaokun_ec *ec, struct
> > > > notifier_block *nb)
> > > > +{
> > > > +     blocking_notifier_chain_unregister(&ec->notifier_list, nb);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(gaokun_ec_unregister_notify);
> > > > +
> > > > +/*
> > > > --------------------------------------------------------------------------
> > > > */
> > > > +/* API for PSY */
> > > > +
> > > > +/**
> > > > + * gaokun_ec_psy_multi_read - Read contiguous registers
> > > > + * @ec: The gaokun_ec structure
> > > > + * @reg: The start register
> > > > + * @resp_len: The number of registers to be read
> > > > + * @resp: The buffer to store response sequence
> > > > + *
> > > > + * Return: 0 on success or negative error code.
> > > > + */
> > > > +int gaokun_ec_psy_multi_read(struct gaokun_ec *ec, u8 reg,
> > > > +                          size_t resp_len, u8 *resp)
> > > > +{
> > > > +     u8 ec_req[] = MKREQ(0x02, EC_READ, 1, 0);
> > > > +     u8 ec_resp[] = MKRESP(1);
> > > > +     int i, ret;
> > > > +
> > > > +     for (i = 0; i < resp_len; ++i, reg++) {
> > > > +             refill_req_byte(ec_req, &reg);
> > > > +             ret = gaokun_ec_read(ec, ec_req, sizeof(ec_resp),
> > > > ec_resp);
> > > > +             if (ret)
> > > > +                     return ret;
> > > > +             extr_resp_byte(&resp[i], ec_resp);
> > > > +     }
> > > > +
> > > > +     return 0;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(gaokun_ec_psy_multi_read);
> > > > +
> > > > +/* Smart charge */
> > > > +
> > > > +/**
> > > > + * gaokun_ec_psy_get_smart_charge - Get smart charge data from EC
> > > > + * @ec: The gaokun_ec structure
> > > > + * @resp: The buffer to store response sequence (mode, delay, start,
> > > > end)
> > > > + *
> > > > + * Return: 0 on success or negative error code.
> > > > + */
> > > > +int gaokun_ec_psy_get_smart_charge(struct gaokun_ec *ec,
> > > > +                                u8 resp[GAOKUN_SMART_CHARGE_DATA_SIZE])
> > > > +{
> > > > +     /* GBCM */
> > > > +     u8 ec_req[] = MKREQ(0x02, 0xE4, 0);
> > > > +     u8 ec_resp[] = MKRESP(GAOKUN_SMART_CHARGE_DATA_SIZE);
> > > > +     int ret;
> > > > +
> > > > +     ret = gaokun_ec_read(ec, ec_req, sizeof(ec_resp), ec_resp);
> > > > +     if (ret)
> > > > +             return ret;
> > > > +
> > > > +     extr_resp(resp, ec_resp, GAOKUN_SMART_CHARGE_DATA_SIZE);
> > > > +
> > > > +     return 0;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(gaokun_ec_psy_get_smart_charge);
> > > > +
> > > > +static inline bool are_thresholds_valid(u8 start, u8 end)
> > > > +{
> > > > +     return end != 0 && start <= end && end <= 100;
> > > 
> > > Why 100 ? Still feels like an arbitrary number.
> > > 
> > > Could you add a comment to explain where 100 comes from ?
> > > 
> > 
> > You may don't get it. It is just a battery percentage, greater than 100 is
> > invalid.
> 
> 100 meaning maximum capacity, good stuff.
> 
> Please use a define with a descriptive name. That way the meaning is obvious.
> 
> In fact if the name of the function related to battery capacity then the
> meaning of the numbers would be more obvious.
> 
> static inline bool validate_battery_threshold_range(u8 start, u8 end) {
> 	return end != 0 && start <= end && end <= 100;
> }

I suggest going with this latter option. 100% tends to be in its literal 
form elsewhere too. I suppose we don't even have a generic define for 
"100%", at least I don't recall coming across one nor found any with a 
quick git grep underneath include/.

But I agree the naming of this function could be improved like you 
suggest.

-- 
 i.

> > 
> > start: The battery percentage at which charging starts (0-100).
> > stop: The battery percentage at which charging stops (1-100).
> 
> Or just add this comment directly above the function.
> 
> ---
> bod
>