Re: [PATCH v4 2/6] platform: arm64: add Lenovo Yoga C630 WOS EC driver
From: Bryan O'Donoghue
Date: Wed May 29 2024 - 04:18:13 EST
On 29/05/2024 00:56, Dmitry Baryshkov wrote:
On Wed, May 29, 2024 at 12:51:04AM +0100, Bryan O'Donoghue wrote:
On 28/05/2024 21:44, Dmitry Baryshkov wrote:
Lenovo Yoga C630 WOS is a laptop using Snapdragon 850 SoC. Like many
laptops it uses embedded controller (EC) to perform various platform
an embedded controller
operations, including, but not limited, to Type-C port control or power
supply handlng.
Add the driver for the EC, that creates devices for UCSI and power
supply devices.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
drivers/platform/arm64/Kconfig | 14 ++
drivers/platform/arm64/Makefile | 1 +
drivers/platform/arm64/lenovo-yoga-c630.c | 279 +++++++++++++++++++++++++
include/linux/platform_data/lenovo-yoga-c630.h | 42 ++++
4 files changed, 336 insertions(+)
diff --git a/drivers/platform/arm64/Kconfig b/drivers/platform/arm64/Kconfig
index 8fdca0f8e909..8c103b3150d1 100644
--- a/drivers/platform/arm64/Kconfig
+++ b/drivers/platform/arm64/Kconfig
@@ -32,4 +32,18 @@ config EC_ACER_ASPIRE1
laptop where this information is not properly exposed via the
standard ACPI devices.
+config EC_LENOVO_YOGA_C630
+ tristate "Lenovo Yoga C630 Embedded Controller driver"
+ depends on I2C
+ help
+ Driver for the Embedded Controller in the Qualcomm Snapdragon-based
+ Lenovo Yoga C630, which provides battery and power adapter
+ information.
+
+ 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.
+
endif # ARM64_PLATFORM_DEVICES
diff --git a/drivers/platform/arm64/Makefile b/drivers/platform/arm64/Makefile
index 4fcc9855579b..b2ae9114fdd8 100644
--- a/drivers/platform/arm64/Makefile
+++ b/drivers/platform/arm64/Makefile
@@ -6,3 +6,4 @@
#
obj-$(CONFIG_EC_ACER_ASPIRE1) += acer-aspire1-ec.o
+obj-$(CONFIG_EC_LENOVO_YOGA_C630) += lenovo-yoga-c630.o
diff --git a/drivers/platform/arm64/lenovo-yoga-c630.c b/drivers/platform/arm64/lenovo-yoga-c630.c
new file mode 100644
index 000000000000..3d1d5acde807
--- /dev/null
+++ b/drivers/platform/arm64/lenovo-yoga-c630.c
@@ -0,0 +1,279 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2022-2024, Linaro Ltd
+ * Authors:
+ * Bjorn Andersson
+ * Dmitry Baryshkov
+ */
+#include <linux/auxiliary_bus.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/notifier.h>
+#include <linux/platform_data/lenovo-yoga-c630.h>
+
+#define LENOVO_EC_RESPONSE_REG 0x01
+#define LENOVO_EC_REQUEST_REG 0x02
+
+#define LENOVO_EC_UCSI_WRITE 0x20
+#define LENOVO_EC_UCSI_READ 0x21
+
+#define LENOVO_EC_READ_REG 0xb0
+#define LENOVO_EC_REQUEST_NEXT_EVENT 0x84
+
+struct yoga_c630_ec {
+ struct i2c_client *client;
+ struct mutex lock;
+ struct blocking_notifier_head notifier_list;
+};
+
+static int yoga_c630_ec_request(struct yoga_c630_ec *ec, u8 *req, size_t req_len,
+ u8 *resp, size_t resp_len)
+{
+ int ret;
+
+ WARN_ON(!mutex_is_locked(&ec->lock));
+
+ ret = i2c_smbus_write_i2c_block_data(ec->client, LENOVO_EC_REQUEST_REG,
+ req_len, req);
+ if (ret < 0)
+ return ret;
+
+ return i2c_smbus_read_i2c_block_data(ec->client, LENOVO_EC_RESPONSE_REG,
+ resp_len, resp);
+}
+
+int yoga_c630_ec_read8(struct yoga_c630_ec *ec, u8 addr)
+{
+ u8 req[2] = { LENOVO_EC_READ_REG, };
+ int ret;
+ u8 val;
+
+ mutex_lock(&ec->lock);
+ req[1] = addr;
+ ret = yoga_c630_ec_request(ec, req, sizeof(req), &val, 1);
+ mutex_unlock(&ec->lock);
+
+ return ret < 0 ? ret : val;
+}
+EXPORT_SYMBOL_GPL(yoga_c630_ec_read8);
+
+int yoga_c630_ec_read16(struct yoga_c630_ec *ec, u8 addr)
+{
+ u8 req[2] = { LENOVO_EC_READ_REG, };
+ int ret;
+ u8 msb;
+ u8 lsb;
+
+ mutex_lock(&ec->lock);
+
+ req[1] = addr;
+ ret = yoga_c630_ec_request(ec, req, sizeof(req), &lsb, 1);
+ if (ret < 0)
+ goto out;
+
+ req[1] = addr + 1;
+ ret = yoga_c630_ec_request(ec, req, sizeof(req), &msb, 1);
+
+out:
+ mutex_unlock(&ec->lock);
+
+ return ret < 0 ? ret : msb << 8 | lsb;
+}
+EXPORT_SYMBOL_GPL(yoga_c630_ec_read16);
+
+u16 yoga_c630_ec_ucsi_get_version(struct yoga_c630_ec *ec)
+{
+ u8 req[3] = { 0xb3, 0xf2, 0x20};
You have a define above for the read_reg and write_reg commands, could you
not define 0xb3 as LENOVO_EC_GET_VERSION ?
All of the other commands here seem to have a named define.
Because unlike other registers it is not clear what other use cases does
0xb3 support
+ int ret;
+ u8 msb;
+ u8 lsb;
+
+ mutex_lock(&ec->lock);
+ ret = yoga_c630_ec_request(ec, req, sizeof(req), &lsb, 1);
+ if (ret < 0)
+ goto out;
+
+ req[2]++;
why not set reg[2] = 0x21;
ack
also is req[2] some kind of address ?
Unfortunately no idea. This is totally based on the AML code in DSDT. I
have no documentation on the EC or its programming interface.
+ ret = yoga_c630_ec_request(ec, req, sizeof(req), &msb, 1);
+
+out:
+ mutex_unlock(&ec->lock);
+
+ return ret < 0 ? ret : msb << 8 | lsb;
+}
+EXPORT_SYMBOL_GPL(yoga_c630_ec_ucsi_get_version);
+
+int yoga_c630_ec_ucsi_write(struct yoga_c630_ec *ec,
+ const u8 req[YOGA_C630_UCSI_WRITE_SIZE])
+{
+ int ret;
+
+ mutex_lock(&ec->lock);
+ ret = i2c_smbus_write_i2c_block_data(ec->client, LENOVO_EC_UCSI_WRITE,
+ YOGA_C630_UCSI_WRITE_SIZE, req);
+ mutex_unlock(&ec->lock);
+
+ return ret < 0 ? ret : 0;
+}
+EXPORT_SYMBOL_GPL(yoga_c630_ec_ucsi_write);
+
+int yoga_c630_ec_ucsi_read(struct yoga_c630_ec *ec,
+ u8 resp[YOGA_C630_UCSI_READ_SIZE])
+{
+ int ret;
+
+ mutex_lock(&ec->lock);
+ ret = i2c_smbus_read_i2c_block_data(ec->client, LENOVO_EC_UCSI_READ,
+ YOGA_C630_UCSI_READ_SIZE, resp);
+ mutex_unlock(&ec->lock);
+
+ return ret < 0 ? ret : 0;
+}
+EXPORT_SYMBOL_GPL(yoga_c630_ec_ucsi_read);
+
+static irqreturn_t yoga_c630_ec_intr(int irq, void *data)
+{
+ u8 req[] = { LENOVO_EC_REQUEST_NEXT_EVENT };
+ struct yoga_c630_ec *ec = data;
+ u8 event;
+ int ret;
+
+ mutex_lock(&ec->lock);
+ ret = yoga_c630_ec_request(ec, req, sizeof(req), &event, 1);
+ mutex_unlock(&ec->lock);
+ if (ret < 0)
+ return IRQ_HANDLED;
+
+ pr_info("NOTIFY %x\n", event);
why not dev_info() ?
Argh, debugging code. I should drop it.
Assuming you do all of that in v5
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>