All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] cros: platform/chrome: Add cros-ec-uart driver for uart support
@ 2020-07-15  8:25 ` Bhanu Prakash Maiya
  0 siblings, 0 replies; 10+ messages in thread
From: Bhanu Prakash Maiya @ 2020-07-15  8:25 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Lee Jones, Furquan Shaikh, Raul E Rangel, Eric Peers,
	Duncan Laurie, Benson Leung, Enric Balletbo i Serra,
	Bhanu Prakash Maiya, Guenter Roeck, Mauro Carvalho Chehab,
	David S . Miller, Rob Herring, Greg Kroah-Hartman, devicetree,
	linux-kernel, kernel test robot, Bhanu Prakash Maiya

From: Bhanu Prakash Maiya <bhanumaiya@chromium.org>

This patch enables uart transport layer for cros_ec framework.
The cros-ec-uart binds with EC device working on uart transport to
send request and receive response.

Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@chromium.org>
Reported-by: kernel test robot <lkp@intel.com>
Change-Id: Icb23b633700f1ef4d123e3f21fd26fad21a3f207
Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@google.com>
---
Changes in v2:
1: Fixed build error on v1.
2: Changed EC timeout for response packet to 3 Sec and added comments.
3: Fixed cros_ec_uart_rx_bytes function to handle rx buffer < size of response header.

 MAINTAINERS                            |   1 +
 drivers/platform/chrome/Kconfig        |  10 +
 drivers/platform/chrome/Makefile       |   1 +
 drivers/platform/chrome/cros_ec_uart.c | 411 +++++++++++++++++++++++++
 4 files changed, 423 insertions(+)
 create mode 100644 drivers/platform/chrome/cros_ec_uart.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 50659d76976b7..aa8e1d121c1d7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4036,6 +4036,7 @@ F:	sound/soc/codecs/cros_ec_codec.*
 CHROMEOS EC SUBDRIVERS
 M:	Benson Leung <bleung@chromium.org>
 M:	Enric Balletbo i Serra <enric.balletbo@collabora.com>
+M:	Bhanu Prakash Maiya <bhanumaiya@chromium.org>
 R:	Guenter Roeck <groeck@chromium.org>
 S:	Maintained
 F:	drivers/power/supply/cros_usbpd-charger.c
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 3822e5e111caa..2082fafe08a6a 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -125,6 +125,16 @@ config CROS_EC_SPI
 	  response time cannot be guaranteed, we support ignoring
 	  'pre-amble' bytes before the response actually starts.
 
+config CROS_EC_UART
+	tristate "ChromeOS Embedded Controller (UART)"
+	depends on CROS_EC && ACPI && SERIAL_DEV_BUS
+	help
+	  If you say Y here, you get support for talking to the ChromeOS EC
+	  through a UART, using a byte-level protocol.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called cros_ec_uart.
+
 config CROS_EC_LPC
 	tristate "ChromeOS Embedded Controller (LPC)"
 	depends on CROS_EC && ACPI && (X86 || COMPILE_TEST)
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 8ed1e33033b38..fc449351fc794 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_CROS_EC_I2C)		+= cros_ec_i2c.o
 obj-$(CONFIG_CROS_EC_ISHTP)		+= cros_ec_ishtp.o
 obj-$(CONFIG_CROS_EC_RPMSG)		+= cros_ec_rpmsg.o
 obj-$(CONFIG_CROS_EC_SPI)		+= cros_ec_spi.o
+obj-$(CONFIG_CROS_EC_UART)		+= cros_ec_uart.o
 cros_ec_lpcs-objs			:= cros_ec_lpc.o cros_ec_lpc_mec.o
 obj-$(CONFIG_CROS_EC_TYPEC)		+= cros_ec_typec.o
 obj-$(CONFIG_CROS_EC_LPC)		+= cros_ec_lpcs.o
diff --git a/drivers/platform/chrome/cros_ec_uart.c b/drivers/platform/chrome/cros_ec_uart.c
new file mode 100644
index 0000000000000..9a0ba884b6812
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_uart.c
@@ -0,0 +1,411 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * UART interface for ChromeOS Embedded Controller
+ *
+ * Copyright 2020 Google LLC.
+ */
+
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/acpi.h>
+#include <linux/of.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
+#include <linux/serdev.h>
+#include <linux/slab.h>
+#include <uapi/linux/sched/types.h>
+
+#include "cros_ec.h"
+
+/*
+ * EC sends contiguous bytes of response packet on UART AP RX.
+ * TTY driver in AP accumulates incoming bytes and calls the registered callback
+ * function. Byte count can range from 1 to Max count supported by TTY driver.
+ * This driver should wait for long time for all callbacks to be processed.
+ * Considering the worst case scenario, wait for ~3 secs. This timeout should
+ * account for max latency and some additional guard time.
+ * In case the packet is received in ms, wait queue will be released and packet
+ * will be processed.
+ */
+#define EC_MSG_DEADLINE_MS		(300 * 10)
+
+/**
+ * struct response_info - Encapsulate EC response related
+ *			information for passing between function
+ *			cros_ec_uart_pkt_xfer() and cros_ec_uart_rx_bytes()
+ *			callback.
+ * @data:		Copy the data received from EC here.
+ * @max_size:		Max size allocated for the @data buffer. If the
+ *			received data exceeds this value, we log an error.
+ * @size:		Actual size of data received from EC. This is also
+ *			used to accumulate byte count with response is received
+ *			in dma chunks.
+ * @exp_len:		Expected bytes of response from EC including header.
+ * @error:		0 for success, negative error code for a failure.
+ * @received:		Set to true on receiving a valid EC response.
+ * @wait_queue:		Wait queue EC response where the cros_ec sends request
+ *			to EC and waits
+ */
+struct response_info {
+	void *data;
+	size_t max_size;
+	size_t size;
+	int error;
+	size_t exp_len;
+	bool received;
+	wait_queue_head_t wait_queue;
+};
+
+/**
+ * struct cros_ec_uart - information about a uart-connected EC
+ *
+ * @serdev_device:	serdev uart device we are connected to.
+ * @baudrate:		UART baudrate of attached EC device.
+ * @flowcontrol:	UART flowcontrol of attached device.
+ * @irq:		Linux IRQ number of associated serial device.
+ * @response:		Response info passing between cros_ec_uart_pkt_xfer()
+ *			and cros_ec_uart_rx_bytes()
+ */
+struct cros_ec_uart {
+	struct serdev_device *serdev;
+	u32 baudrate;
+	u8  flowcontrol;
+	u32 irq;
+	struct response_info response;
+};
+
+static int cros_ec_uart_rx_bytes(struct serdev_device *serdev,
+				 const u8 *data,
+				 size_t count)
+{
+	struct ec_host_response *response;
+	struct cros_ec_device *ec_dev =   serdev_device_get_drvdata(serdev);
+	struct cros_ec_uart *ec_uart = ec_dev->priv;
+
+	/* Check if bytes were sent out of band */
+	if (!ec_uart->response.data) {
+		/* Discard all bytes */
+		return count;
+	}
+
+	/*
+	 * Check if incoming bytes + response.size are less than allocated
+	 * buffer in din by cros_ec. This will ensure that if EC sends more
+	 * bytes than max_size, waiting process will be notified with an error.
+	 */
+	if (ec_uart->response.size + count <= ec_uart->response.max_size) {
+		/* Copy bytes in data in buffer */
+		memcpy((void *)ec_uart->response.data + ec_uart->response.size,
+		       (void *)data, count);
+
+		/* Add incoming bytes in size */
+		ec_uart->response.size += count;
+
+		/*
+		 * Read data_len if we received response header and if exp_len
+		 * was not read before.
+		 */
+		if (ec_uart->response.size >= sizeof(*response) &&
+		    ec_uart->response.exp_len == 0) {
+			/* Get expected response length from response header */
+			response = (struct ec_host_response *)
+							ec_uart->response.data;
+
+			ec_uart->response.exp_len = response->data_len +
+				sizeof(*response);
+		}
+
+		/*
+		 * If driver received response header and payload from EC,
+		 * Wake up the wait queue.
+		 */
+		if (ec_uart->response.size >= sizeof(*response) &&
+		    ec_uart->response.size == ec_uart->response.exp_len) {
+			/* Set flag before waking up the caller */
+			ec_uart->response.received = true;
+
+			/* Wake the calling thread */
+			wake_up_interruptible(&ec_uart->response.wait_queue);
+		}
+	} else {
+		/* Received bytes are more the allocated buffer*/
+		ec_uart->response.error = -EMSGSIZE;
+
+		/* Wake the calling thread */
+		wake_up_interruptible(&ec_uart->response.wait_queue);
+	}
+
+	return count;
+}
+
+static int cros_ec_uart_pkt_xfer(struct cros_ec_device *ec_dev,
+				 struct cros_ec_command *ec_msg)
+{
+	struct cros_ec_uart *ec_uart = ec_dev->priv;
+	struct serdev_device *serdev = ec_uart->serdev;
+	struct ec_host_response *response;
+	unsigned int len;
+	int ret, i;
+	u8 sum = 0;
+
+	/* Prepare an outgoing message in the output buffer */
+	len = cros_ec_prepare_tx(ec_dev, ec_msg);
+	dev_dbg(ec_dev->dev, "Prepared len=%d\n", len);
+
+	/* Setup for incoming response */
+	ec_uart->response.data = ec_dev->din;
+	ec_uart->response.max_size = ec_dev->din_size;
+	ec_uart->response.size = 0;
+	ec_uart->response.error = 0;
+	ec_uart->response.exp_len = 0;
+	ec_uart->response.received = false;
+
+	/* Write serial device buffer */
+	ret = serdev_device_write_buf(serdev, ec_dev->dout, len);
+	if (ret < len) {
+		dev_err(&serdev->dev,
+			"Unable to write data to serial device %s",
+			dev_name(&serdev->dev));
+
+		/* Return EIO as controller had issues writing buffer */
+		ret = -EIO;
+		goto exit;
+	}
+
+	/* Once request is successfully sent to EC, wait to wait_queue */
+	wait_event_interruptible_timeout(ec_uart->response.wait_queue,
+					 ec_uart->response.received,
+					 msecs_to_jiffies(EC_MSG_DEADLINE_MS));
+
+	/* Check if wait_queue was interrupted due to an error */
+	if (ec_uart->response.error < 0) {
+		dev_warn(&serdev->dev, "Response error detected.\n");
+
+		ret = ec_uart->response.error;
+		goto exit;
+	}
+
+	/* Check if valid response was received or there was a timeout */
+	if (!ec_uart->response.received) {
+		dev_warn(&serdev->dev, "EC failed to respond in time.\n");
+
+		ret = -ETIMEDOUT;
+		goto exit;
+	}
+
+	/* Check response error code */
+	response = (struct ec_host_response *)ec_dev->din;
+	ec_msg->result = response->result;
+
+	/* Check if received response is longer than expected */
+	if (response->data_len > ec_msg->insize) {
+		dev_err(ec_dev->dev, "Resp too long (%d bytes, expected %d)",
+			response->data_len,
+			ec_msg->insize);
+		ret = -ENOSPC;
+		goto exit;
+	}
+
+	/* Copy response packet to ec_msg data buffer */
+	memcpy(ec_msg->data,
+	       ec_dev->din + sizeof(*response),
+	       response->data_len);
+
+	/* Add all response header bytes for checksum calculation */
+	for (i = 0; i < sizeof(*response); i++)
+		sum += ec_dev->din[i];
+
+	/* Copy response packet payload and compute checksum */
+	for (i = 0; i < response->data_len; i++)
+		sum += ec_msg->data[i];
+
+	if (sum) {
+		dev_err(ec_dev->dev,
+			"Bad packet checksum calculated %x\n",
+			sum);
+		ret = -EBADMSG;
+		goto exit;
+	}
+
+	/* Return data_len to cros_ec */
+	ret = response->data_len;
+
+exit:
+	/* Reset ec_uart */
+	ec_uart->response.data = NULL;
+	ec_uart->response.max_size = 0;
+	ec_uart->response.size = 0;
+	ec_uart->response.error = 0;
+	ec_uart->response.exp_len = 0;
+	ec_uart->response.received = false;
+
+	if (ec_msg->command == EC_CMD_REBOOT_EC)
+		msleep(EC_REBOOT_DELAY_MS);
+
+	return ret;
+}
+
+static int cros_ec_uart_resource(struct acpi_resource *ares, void *data)
+{
+	struct cros_ec_uart *ec_uart = data;
+	struct acpi_resource_uart_serialbus *sb;
+
+	switch (ares->type) {
+	case ACPI_RESOURCE_TYPE_SERIAL_BUS:
+		sb = &ares->data.uart_serial_bus;
+		if (sb->type == ACPI_RESOURCE_SERIAL_TYPE_UART) {
+			ec_uart->baudrate = sb->default_baud_rate;
+			dev_dbg(&ec_uart->serdev->dev, "Baudrate %d\n",
+				ec_uart->baudrate);
+
+			ec_uart->flowcontrol = sb->flow_control;
+			dev_dbg(&ec_uart->serdev->dev, "Flow control %d\n",
+				ec_uart->flowcontrol);
+		}
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int cros_ec_uart_acpi_probe(struct cros_ec_uart *ec_uart)
+{
+	LIST_HEAD(resources);
+	struct acpi_device *adev = ACPI_COMPANION(&ec_uart->serdev->dev);
+	int ret;
+
+	/* Retrieve UART ACPI info */
+	ret = acpi_dev_get_resources(adev, &resources,
+				     cros_ec_uart_resource, ec_uart);
+	if (ret < 0)
+		return ret;
+
+	acpi_dev_free_resource_list(&resources);
+
+	/* Retrieve GpioInt and translate it to Linux IRQ number */
+	ret = acpi_dev_gpio_irq_get(adev, 0);
+	if (ret < 0)
+		return ret;
+
+	ec_uart->irq = ret;
+	dev_dbg(&ec_uart->serdev->dev, "IRQ number %d\n", ec_uart->irq);
+
+	return 0;
+}
+
+static const struct serdev_device_ops cros_ec_uart_client_ops = {
+	.receive_buf = cros_ec_uart_rx_bytes,
+};
+
+static int cros_ec_uart_probe(struct serdev_device *serdev)
+{
+	struct device *dev = &serdev->dev;
+	struct cros_ec_device *ec_dev;
+	struct cros_ec_uart *ec_uart;
+	int ret;
+
+	ec_uart = devm_kzalloc(dev, sizeof(*ec_uart), GFP_KERNEL);
+	if (!ec_uart)
+		return -ENOMEM;
+
+	ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL);
+	if (!ec_dev)
+		return -ENOMEM;
+
+	ec_uart->serdev = serdev;
+
+	/* Open the serial device */
+	ret = devm_serdev_device_open(dev, ec_uart->serdev);
+	if (ret) {
+		dev_err(dev, "Unable to open UART device %s",
+			dev_name(&serdev->dev));
+		return ret;
+	}
+
+	serdev_device_set_drvdata(serdev, ec_dev);
+
+	serdev_device_set_client_ops(serdev, &cros_ec_uart_client_ops);
+
+	/* Initialize wait queue */
+	init_waitqueue_head(&ec_uart->response.wait_queue);
+
+	ret = cros_ec_uart_acpi_probe(ec_uart);
+	if (ret < 0) {
+		dev_err(dev, "Failed to get ACPI info (%d)", ret);
+		return ret;
+	}
+
+	/* Set baud rate of serial device */
+	ret = serdev_device_set_baudrate(serdev, ec_uart->baudrate);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set up host baud rate (%d)", ret);
+		return ret;
+	}
+
+	/* Set flow control of serial device */
+	serdev_device_set_flow_control(serdev, ec_uart->flowcontrol);
+
+	/* Initialize ec_dev for cros_ec  */
+	ec_dev->phys_name = dev_name(&ec_uart->serdev->dev);
+	ec_dev->dev = dev;
+	ec_dev->priv = ec_uart;
+	ec_dev->irq = ec_uart->irq;
+	ec_dev->cmd_xfer = NULL;
+	ec_dev->pkt_xfer = cros_ec_uart_pkt_xfer;
+	ec_dev->din_size = sizeof(struct ec_host_response) +
+			   sizeof(struct ec_response_get_protocol_info);
+	ec_dev->dout_size = sizeof(struct ec_host_request);
+
+	/* Register a new cros_ec device */
+	return cros_ec_register(ec_dev);
+}
+
+static void cros_ec_uart_remove(struct serdev_device *serdev)
+{
+	struct cros_ec_device *ec_dev = serdev_device_get_drvdata(serdev);
+
+	cros_ec_unregister(ec_dev);
+};
+
+static int __maybe_unused cros_ec_uart_suspend(struct device *dev)
+{
+	struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
+
+	return cros_ec_suspend(ec_dev);
+}
+
+static int __maybe_unused cros_ec_uart_resume(struct device *dev)
+{
+	struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
+
+	return cros_ec_resume(ec_dev);
+}
+
+static SIMPLE_DEV_PM_OPS(cros_ec_uart_pm_ops, cros_ec_uart_suspend,
+			 cros_ec_uart_resume);
+
+static const struct of_device_id cros_ec_uart_of_match[] = {
+	{ .compatible = "google,cros-ec-uart" },
+	{}
+};
+
+static struct serdev_device_driver cros_ec_uart_driver = {
+	.driver	= {
+		.name	= "cros-ec-uart",
+		.of_match_table = cros_ec_uart_of_match,
+		.pm	= &cros_ec_uart_pm_ops,
+	},
+	.probe		= cros_ec_uart_probe,
+	.remove		= cros_ec_uart_remove,
+};
+
+module_serdev_device_driver(cros_ec_uart_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("UART interface for ChromeOS Embedded Controller");
+MODULE_AUTHOR("Bhanu Prakash Maiya <bhanumaiya@chromium.org>");
-- 
2.27.0.389.gc38d7665816-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 1/2] cros: platform/chrome: Add cros-ec-uart driver for uart support
@ 2020-07-15  8:25 ` Bhanu Prakash Maiya
  0 siblings, 0 replies; 10+ messages in thread
From: Bhanu Prakash Maiya @ 2020-07-15  8:25 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Rob Herring, Mauro Carvalho Chehab, Duncan Laurie,
	kernel test robot, devicetree, Furquan Shaikh, Lee Jones,
	linux-kernel, Bhanu Prakash Maiya, David S . Miller,
	Raul E Rangel, Greg Kroah-Hartman, Enric Balletbo i Serra,
	Guenter Roeck, Benson Leung, Eric Peers, Bhanu Prakash Maiya

From: Bhanu Prakash Maiya <bhanumaiya@chromium.org>

This patch enables uart transport layer for cros_ec framework.
The cros-ec-uart binds with EC device working on uart transport to
send request and receive response.

Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@chromium.org>
Reported-by: kernel test robot <lkp@intel.com>
Change-Id: Icb23b633700f1ef4d123e3f21fd26fad21a3f207
Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@google.com>
---
Changes in v2:
1: Fixed build error on v1.
2: Changed EC timeout for response packet to 3 Sec and added comments.
3: Fixed cros_ec_uart_rx_bytes function to handle rx buffer < size of response header.

 MAINTAINERS                            |   1 +
 drivers/platform/chrome/Kconfig        |  10 +
 drivers/platform/chrome/Makefile       |   1 +
 drivers/platform/chrome/cros_ec_uart.c | 411 +++++++++++++++++++++++++
 4 files changed, 423 insertions(+)
 create mode 100644 drivers/platform/chrome/cros_ec_uart.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 50659d76976b7..aa8e1d121c1d7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4036,6 +4036,7 @@ F:	sound/soc/codecs/cros_ec_codec.*
 CHROMEOS EC SUBDRIVERS
 M:	Benson Leung <bleung@chromium.org>
 M:	Enric Balletbo i Serra <enric.balletbo@collabora.com>
+M:	Bhanu Prakash Maiya <bhanumaiya@chromium.org>
 R:	Guenter Roeck <groeck@chromium.org>
 S:	Maintained
 F:	drivers/power/supply/cros_usbpd-charger.c
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 3822e5e111caa..2082fafe08a6a 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -125,6 +125,16 @@ config CROS_EC_SPI
 	  response time cannot be guaranteed, we support ignoring
 	  'pre-amble' bytes before the response actually starts.
 
+config CROS_EC_UART
+	tristate "ChromeOS Embedded Controller (UART)"
+	depends on CROS_EC && ACPI && SERIAL_DEV_BUS
+	help
+	  If you say Y here, you get support for talking to the ChromeOS EC
+	  through a UART, using a byte-level protocol.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called cros_ec_uart.
+
 config CROS_EC_LPC
 	tristate "ChromeOS Embedded Controller (LPC)"
 	depends on CROS_EC && ACPI && (X86 || COMPILE_TEST)
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 8ed1e33033b38..fc449351fc794 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_CROS_EC_I2C)		+= cros_ec_i2c.o
 obj-$(CONFIG_CROS_EC_ISHTP)		+= cros_ec_ishtp.o
 obj-$(CONFIG_CROS_EC_RPMSG)		+= cros_ec_rpmsg.o
 obj-$(CONFIG_CROS_EC_SPI)		+= cros_ec_spi.o
+obj-$(CONFIG_CROS_EC_UART)		+= cros_ec_uart.o
 cros_ec_lpcs-objs			:= cros_ec_lpc.o cros_ec_lpc_mec.o
 obj-$(CONFIG_CROS_EC_TYPEC)		+= cros_ec_typec.o
 obj-$(CONFIG_CROS_EC_LPC)		+= cros_ec_lpcs.o
diff --git a/drivers/platform/chrome/cros_ec_uart.c b/drivers/platform/chrome/cros_ec_uart.c
new file mode 100644
index 0000000000000..9a0ba884b6812
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_uart.c
@@ -0,0 +1,411 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * UART interface for ChromeOS Embedded Controller
+ *
+ * Copyright 2020 Google LLC.
+ */
+
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/acpi.h>
+#include <linux/of.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
+#include <linux/serdev.h>
+#include <linux/slab.h>
+#include <uapi/linux/sched/types.h>
+
+#include "cros_ec.h"
+
+/*
+ * EC sends contiguous bytes of response packet on UART AP RX.
+ * TTY driver in AP accumulates incoming bytes and calls the registered callback
+ * function. Byte count can range from 1 to Max count supported by TTY driver.
+ * This driver should wait for long time for all callbacks to be processed.
+ * Considering the worst case scenario, wait for ~3 secs. This timeout should
+ * account for max latency and some additional guard time.
+ * In case the packet is received in ms, wait queue will be released and packet
+ * will be processed.
+ */
+#define EC_MSG_DEADLINE_MS		(300 * 10)
+
+/**
+ * struct response_info - Encapsulate EC response related
+ *			information for passing between function
+ *			cros_ec_uart_pkt_xfer() and cros_ec_uart_rx_bytes()
+ *			callback.
+ * @data:		Copy the data received from EC here.
+ * @max_size:		Max size allocated for the @data buffer. If the
+ *			received data exceeds this value, we log an error.
+ * @size:		Actual size of data received from EC. This is also
+ *			used to accumulate byte count with response is received
+ *			in dma chunks.
+ * @exp_len:		Expected bytes of response from EC including header.
+ * @error:		0 for success, negative error code for a failure.
+ * @received:		Set to true on receiving a valid EC response.
+ * @wait_queue:		Wait queue EC response where the cros_ec sends request
+ *			to EC and waits
+ */
+struct response_info {
+	void *data;
+	size_t max_size;
+	size_t size;
+	int error;
+	size_t exp_len;
+	bool received;
+	wait_queue_head_t wait_queue;
+};
+
+/**
+ * struct cros_ec_uart - information about a uart-connected EC
+ *
+ * @serdev_device:	serdev uart device we are connected to.
+ * @baudrate:		UART baudrate of attached EC device.
+ * @flowcontrol:	UART flowcontrol of attached device.
+ * @irq:		Linux IRQ number of associated serial device.
+ * @response:		Response info passing between cros_ec_uart_pkt_xfer()
+ *			and cros_ec_uart_rx_bytes()
+ */
+struct cros_ec_uart {
+	struct serdev_device *serdev;
+	u32 baudrate;
+	u8  flowcontrol;
+	u32 irq;
+	struct response_info response;
+};
+
+static int cros_ec_uart_rx_bytes(struct serdev_device *serdev,
+				 const u8 *data,
+				 size_t count)
+{
+	struct ec_host_response *response;
+	struct cros_ec_device *ec_dev =   serdev_device_get_drvdata(serdev);
+	struct cros_ec_uart *ec_uart = ec_dev->priv;
+
+	/* Check if bytes were sent out of band */
+	if (!ec_uart->response.data) {
+		/* Discard all bytes */
+		return count;
+	}
+
+	/*
+	 * Check if incoming bytes + response.size are less than allocated
+	 * buffer in din by cros_ec. This will ensure that if EC sends more
+	 * bytes than max_size, waiting process will be notified with an error.
+	 */
+	if (ec_uart->response.size + count <= ec_uart->response.max_size) {
+		/* Copy bytes in data in buffer */
+		memcpy((void *)ec_uart->response.data + ec_uart->response.size,
+		       (void *)data, count);
+
+		/* Add incoming bytes in size */
+		ec_uart->response.size += count;
+
+		/*
+		 * Read data_len if we received response header and if exp_len
+		 * was not read before.
+		 */
+		if (ec_uart->response.size >= sizeof(*response) &&
+		    ec_uart->response.exp_len == 0) {
+			/* Get expected response length from response header */
+			response = (struct ec_host_response *)
+							ec_uart->response.data;
+
+			ec_uart->response.exp_len = response->data_len +
+				sizeof(*response);
+		}
+
+		/*
+		 * If driver received response header and payload from EC,
+		 * Wake up the wait queue.
+		 */
+		if (ec_uart->response.size >= sizeof(*response) &&
+		    ec_uart->response.size == ec_uart->response.exp_len) {
+			/* Set flag before waking up the caller */
+			ec_uart->response.received = true;
+
+			/* Wake the calling thread */
+			wake_up_interruptible(&ec_uart->response.wait_queue);
+		}
+	} else {
+		/* Received bytes are more the allocated buffer*/
+		ec_uart->response.error = -EMSGSIZE;
+
+		/* Wake the calling thread */
+		wake_up_interruptible(&ec_uart->response.wait_queue);
+	}
+
+	return count;
+}
+
+static int cros_ec_uart_pkt_xfer(struct cros_ec_device *ec_dev,
+				 struct cros_ec_command *ec_msg)
+{
+	struct cros_ec_uart *ec_uart = ec_dev->priv;
+	struct serdev_device *serdev = ec_uart->serdev;
+	struct ec_host_response *response;
+	unsigned int len;
+	int ret, i;
+	u8 sum = 0;
+
+	/* Prepare an outgoing message in the output buffer */
+	len = cros_ec_prepare_tx(ec_dev, ec_msg);
+	dev_dbg(ec_dev->dev, "Prepared len=%d\n", len);
+
+	/* Setup for incoming response */
+	ec_uart->response.data = ec_dev->din;
+	ec_uart->response.max_size = ec_dev->din_size;
+	ec_uart->response.size = 0;
+	ec_uart->response.error = 0;
+	ec_uart->response.exp_len = 0;
+	ec_uart->response.received = false;
+
+	/* Write serial device buffer */
+	ret = serdev_device_write_buf(serdev, ec_dev->dout, len);
+	if (ret < len) {
+		dev_err(&serdev->dev,
+			"Unable to write data to serial device %s",
+			dev_name(&serdev->dev));
+
+		/* Return EIO as controller had issues writing buffer */
+		ret = -EIO;
+		goto exit;
+	}
+
+	/* Once request is successfully sent to EC, wait to wait_queue */
+	wait_event_interruptible_timeout(ec_uart->response.wait_queue,
+					 ec_uart->response.received,
+					 msecs_to_jiffies(EC_MSG_DEADLINE_MS));
+
+	/* Check if wait_queue was interrupted due to an error */
+	if (ec_uart->response.error < 0) {
+		dev_warn(&serdev->dev, "Response error detected.\n");
+
+		ret = ec_uart->response.error;
+		goto exit;
+	}
+
+	/* Check if valid response was received or there was a timeout */
+	if (!ec_uart->response.received) {
+		dev_warn(&serdev->dev, "EC failed to respond in time.\n");
+
+		ret = -ETIMEDOUT;
+		goto exit;
+	}
+
+	/* Check response error code */
+	response = (struct ec_host_response *)ec_dev->din;
+	ec_msg->result = response->result;
+
+	/* Check if received response is longer than expected */
+	if (response->data_len > ec_msg->insize) {
+		dev_err(ec_dev->dev, "Resp too long (%d bytes, expected %d)",
+			response->data_len,
+			ec_msg->insize);
+		ret = -ENOSPC;
+		goto exit;
+	}
+
+	/* Copy response packet to ec_msg data buffer */
+	memcpy(ec_msg->data,
+	       ec_dev->din + sizeof(*response),
+	       response->data_len);
+
+	/* Add all response header bytes for checksum calculation */
+	for (i = 0; i < sizeof(*response); i++)
+		sum += ec_dev->din[i];
+
+	/* Copy response packet payload and compute checksum */
+	for (i = 0; i < response->data_len; i++)
+		sum += ec_msg->data[i];
+
+	if (sum) {
+		dev_err(ec_dev->dev,
+			"Bad packet checksum calculated %x\n",
+			sum);
+		ret = -EBADMSG;
+		goto exit;
+	}
+
+	/* Return data_len to cros_ec */
+	ret = response->data_len;
+
+exit:
+	/* Reset ec_uart */
+	ec_uart->response.data = NULL;
+	ec_uart->response.max_size = 0;
+	ec_uart->response.size = 0;
+	ec_uart->response.error = 0;
+	ec_uart->response.exp_len = 0;
+	ec_uart->response.received = false;
+
+	if (ec_msg->command == EC_CMD_REBOOT_EC)
+		msleep(EC_REBOOT_DELAY_MS);
+
+	return ret;
+}
+
+static int cros_ec_uart_resource(struct acpi_resource *ares, void *data)
+{
+	struct cros_ec_uart *ec_uart = data;
+	struct acpi_resource_uart_serialbus *sb;
+
+	switch (ares->type) {
+	case ACPI_RESOURCE_TYPE_SERIAL_BUS:
+		sb = &ares->data.uart_serial_bus;
+		if (sb->type == ACPI_RESOURCE_SERIAL_TYPE_UART) {
+			ec_uart->baudrate = sb->default_baud_rate;
+			dev_dbg(&ec_uart->serdev->dev, "Baudrate %d\n",
+				ec_uart->baudrate);
+
+			ec_uart->flowcontrol = sb->flow_control;
+			dev_dbg(&ec_uart->serdev->dev, "Flow control %d\n",
+				ec_uart->flowcontrol);
+		}
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int cros_ec_uart_acpi_probe(struct cros_ec_uart *ec_uart)
+{
+	LIST_HEAD(resources);
+	struct acpi_device *adev = ACPI_COMPANION(&ec_uart->serdev->dev);
+	int ret;
+
+	/* Retrieve UART ACPI info */
+	ret = acpi_dev_get_resources(adev, &resources,
+				     cros_ec_uart_resource, ec_uart);
+	if (ret < 0)
+		return ret;
+
+	acpi_dev_free_resource_list(&resources);
+
+	/* Retrieve GpioInt and translate it to Linux IRQ number */
+	ret = acpi_dev_gpio_irq_get(adev, 0);
+	if (ret < 0)
+		return ret;
+
+	ec_uart->irq = ret;
+	dev_dbg(&ec_uart->serdev->dev, "IRQ number %d\n", ec_uart->irq);
+
+	return 0;
+}
+
+static const struct serdev_device_ops cros_ec_uart_client_ops = {
+	.receive_buf = cros_ec_uart_rx_bytes,
+};
+
+static int cros_ec_uart_probe(struct serdev_device *serdev)
+{
+	struct device *dev = &serdev->dev;
+	struct cros_ec_device *ec_dev;
+	struct cros_ec_uart *ec_uart;
+	int ret;
+
+	ec_uart = devm_kzalloc(dev, sizeof(*ec_uart), GFP_KERNEL);
+	if (!ec_uart)
+		return -ENOMEM;
+
+	ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL);
+	if (!ec_dev)
+		return -ENOMEM;
+
+	ec_uart->serdev = serdev;
+
+	/* Open the serial device */
+	ret = devm_serdev_device_open(dev, ec_uart->serdev);
+	if (ret) {
+		dev_err(dev, "Unable to open UART device %s",
+			dev_name(&serdev->dev));
+		return ret;
+	}
+
+	serdev_device_set_drvdata(serdev, ec_dev);
+
+	serdev_device_set_client_ops(serdev, &cros_ec_uart_client_ops);
+
+	/* Initialize wait queue */
+	init_waitqueue_head(&ec_uart->response.wait_queue);
+
+	ret = cros_ec_uart_acpi_probe(ec_uart);
+	if (ret < 0) {
+		dev_err(dev, "Failed to get ACPI info (%d)", ret);
+		return ret;
+	}
+
+	/* Set baud rate of serial device */
+	ret = serdev_device_set_baudrate(serdev, ec_uart->baudrate);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set up host baud rate (%d)", ret);
+		return ret;
+	}
+
+	/* Set flow control of serial device */
+	serdev_device_set_flow_control(serdev, ec_uart->flowcontrol);
+
+	/* Initialize ec_dev for cros_ec  */
+	ec_dev->phys_name = dev_name(&ec_uart->serdev->dev);
+	ec_dev->dev = dev;
+	ec_dev->priv = ec_uart;
+	ec_dev->irq = ec_uart->irq;
+	ec_dev->cmd_xfer = NULL;
+	ec_dev->pkt_xfer = cros_ec_uart_pkt_xfer;
+	ec_dev->din_size = sizeof(struct ec_host_response) +
+			   sizeof(struct ec_response_get_protocol_info);
+	ec_dev->dout_size = sizeof(struct ec_host_request);
+
+	/* Register a new cros_ec device */
+	return cros_ec_register(ec_dev);
+}
+
+static void cros_ec_uart_remove(struct serdev_device *serdev)
+{
+	struct cros_ec_device *ec_dev = serdev_device_get_drvdata(serdev);
+
+	cros_ec_unregister(ec_dev);
+};
+
+static int __maybe_unused cros_ec_uart_suspend(struct device *dev)
+{
+	struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
+
+	return cros_ec_suspend(ec_dev);
+}
+
+static int __maybe_unused cros_ec_uart_resume(struct device *dev)
+{
+	struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
+
+	return cros_ec_resume(ec_dev);
+}
+
+static SIMPLE_DEV_PM_OPS(cros_ec_uart_pm_ops, cros_ec_uart_suspend,
+			 cros_ec_uart_resume);
+
+static const struct of_device_id cros_ec_uart_of_match[] = {
+	{ .compatible = "google,cros-ec-uart" },
+	{}
+};
+
+static struct serdev_device_driver cros_ec_uart_driver = {
+	.driver	= {
+		.name	= "cros-ec-uart",
+		.of_match_table = cros_ec_uart_of_match,
+		.pm	= &cros_ec_uart_pm_ops,
+	},
+	.probe		= cros_ec_uart_probe,
+	.remove		= cros_ec_uart_remove,
+};
+
+module_serdev_device_driver(cros_ec_uart_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("UART interface for ChromeOS Embedded Controller");
+MODULE_AUTHOR("Bhanu Prakash Maiya <bhanumaiya@chromium.org>");
-- 
2.27.0.389.gc38d7665816-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/2] dt-bindings: mfd: Add DT compatible string "google,cros_ec_uart"
  2020-07-15  8:25 ` Bhanu Prakash Maiya
@ 2020-07-15  8:25   ` Bhanu Prakash Maiya
  -1 siblings, 0 replies; 10+ messages in thread
From: Bhanu Prakash Maiya @ 2020-07-15  8:25 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Lee Jones, Furquan Shaikh, Raul E Rangel, Eric Peers,
	Duncan Laurie, Benson Leung, Enric Balletbo i Serra,
	Bhanu Prakash Maiya, Guenter Roeck, Mauro Carvalho Chehab,
	David S . Miller, Rob Herring, Greg Kroah-Hartman, devicetree,
	linux-kernel, Bhanu Prakash Maiya

From: Bhanu Prakash Maiya <bhanumaiya@chromium.org>

Add DT compatible string in
Documentation/devicetree/bindings/mfd/cros_ec.txt

Series-to: LKML <linux-kernel@vger.kernel.org>
Series-cc: Raul E Rangel <rrangel@chromium.org>, Furquan Shaikh <furquan@chromium.org>, Duncan Laurie <dlaurie@google.com>, Eric Peers <epeers@google.com>, Benson Leung <bleung@chromium.org>, Enric Balletbo i Serra <enric.balletbo@collabora.com>, Guenter Roeck <groeck@chromium.org>, linux-kernel@vger.kernel.org, Lee Jones <lee.jones@linaro.org>, Rob Herring <robh+dt@kernel.org>, devicetree@vger.kernel.org

Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@chromium.org>
Change-Id: Icfeab15fa04daaffc61280faf5a75cd9b23ee822
Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@google.com>
---
 Documentation/devicetree/bindings/mfd/cros-ec.txt | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/cros-ec.txt b/Documentation/devicetree/bindings/mfd/cros-ec.txt
index 4860eabd0f729..ec8c5d7ecc266 100644
--- a/Documentation/devicetree/bindings/mfd/cros-ec.txt
+++ b/Documentation/devicetree/bindings/mfd/cros-ec.txt
@@ -3,7 +3,7 @@ ChromeOS Embedded Controller
 Google's ChromeOS EC is a Cortex-M device which talks to the AP and
 implements various function such as keyboard and battery charging.
 
-The EC can be connect through various means (I2C, SPI, LPC, RPMSG) and the
+The EC can be connect through various means (I2C, SPI, UART, LPC, RPMSG) and the
 compatible string used depends on the interface. Each connection method has
 its own driver which connects to the top level interface-agnostic EC driver.
 Other Linux driver (such as cros-ec-keyb for the matrix keyboard) connect to
@@ -17,6 +17,10 @@ Required properties (SPI):
 - compatible: "google,cros-ec-spi"
 - reg: SPI chip select
 
+Required properties (UART):
+- compatible: "google,cros-ec-uart"
+- reg: UART baudrate, flowcontrol
+
 Required properties (RPMSG):
 - compatible: "google,cros-ec-rpmsg"
 
@@ -72,5 +76,6 @@ spi@131b0000 {
 	};
 };
 
-
 Example for LPC is not supplied as it is not yet implemented.
+
+Example for UART is not supplied as it is not yet implemented.
-- 
2.27.0.389.gc38d7665816-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/2] dt-bindings: mfd: Add DT compatible string "google, cros_ec_uart"
@ 2020-07-15  8:25   ` Bhanu Prakash Maiya
  0 siblings, 0 replies; 10+ messages in thread
From: Bhanu Prakash Maiya @ 2020-07-15  8:25 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Rob Herring, Mauro Carvalho Chehab, Duncan Laurie, devicetree,
	Furquan Shaikh, Lee Jones, linux-kernel, Bhanu Prakash Maiya,
	David S . Miller, Raul E Rangel, Greg Kroah-Hartman,
	Enric Balletbo i Serra, Guenter Roeck, Benson Leung, Eric Peers,
	Bhanu Prakash Maiya

From: Bhanu Prakash Maiya <bhanumaiya@chromium.org>

Add DT compatible string in
Documentation/devicetree/bindings/mfd/cros_ec.txt

Series-to: LKML <linux-kernel@vger.kernel.org>
Series-cc: Raul E Rangel <rrangel@chromium.org>, Furquan Shaikh <furquan@chromium.org>, Duncan Laurie <dlaurie@google.com>, Eric Peers <epeers@google.com>, Benson Leung <bleung@chromium.org>, Enric Balletbo i Serra <enric.balletbo@collabora.com>, Guenter Roeck <groeck@chromium.org>, linux-kernel@vger.kernel.org, Lee Jones <lee.jones@linaro.org>, Rob Herring <robh+dt@kernel.org>, devicetree@vger.kernel.org

Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@chromium.org>
Change-Id: Icfeab15fa04daaffc61280faf5a75cd9b23ee822
Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@google.com>
---
 Documentation/devicetree/bindings/mfd/cros-ec.txt | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/cros-ec.txt b/Documentation/devicetree/bindings/mfd/cros-ec.txt
index 4860eabd0f729..ec8c5d7ecc266 100644
--- a/Documentation/devicetree/bindings/mfd/cros-ec.txt
+++ b/Documentation/devicetree/bindings/mfd/cros-ec.txt
@@ -3,7 +3,7 @@ ChromeOS Embedded Controller
 Google's ChromeOS EC is a Cortex-M device which talks to the AP and
 implements various function such as keyboard and battery charging.
 
-The EC can be connect through various means (I2C, SPI, LPC, RPMSG) and the
+The EC can be connect through various means (I2C, SPI, UART, LPC, RPMSG) and the
 compatible string used depends on the interface. Each connection method has
 its own driver which connects to the top level interface-agnostic EC driver.
 Other Linux driver (such as cros-ec-keyb for the matrix keyboard) connect to
@@ -17,6 +17,10 @@ Required properties (SPI):
 - compatible: "google,cros-ec-spi"
 - reg: SPI chip select
 
+Required properties (UART):
+- compatible: "google,cros-ec-uart"
+- reg: UART baudrate, flowcontrol
+
 Required properties (RPMSG):
 - compatible: "google,cros-ec-rpmsg"
 
@@ -72,5 +76,6 @@ spi@131b0000 {
 	};
 };
 
-
 Example for LPC is not supplied as it is not yet implemented.
+
+Example for UART is not supplied as it is not yet implemented.
-- 
2.27.0.389.gc38d7665816-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] dt-bindings: mfd: Add DT compatible string "google,cros_ec_uart"
  2020-07-15  8:25   ` [PATCH v2 2/2] dt-bindings: mfd: Add DT compatible string "google, cros_ec_uart" Bhanu Prakash Maiya
@ 2020-07-15 15:40     ` Enric Balletbo i Serra
  -1 siblings, 0 replies; 10+ messages in thread
From: Enric Balletbo i Serra @ 2020-07-15 15:40 UTC (permalink / raw)
  To: Bhanu Prakash Maiya, linux-arm-kernel
  Cc: Lee Jones, Furquan Shaikh, Raul E Rangel, Eric Peers,
	Duncan Laurie, Benson Leung, Guenter Roeck,
	Mauro Carvalho Chehab, David S . Miller, Rob Herring,
	Greg Kroah-Hartman, devicetree, linux-kernel,
	Bhanu Prakash Maiya

Hi bhanu,

Thank you for your patch. This patch has some style problems, please make sure
to fix and resent the patch.

On 15/7/20 10:25, Bhanu Prakash Maiya wrote:
> From: Bhanu Prakash Maiya <bhanumaiya@chromium.org>
> 
> Add DT compatible string in
> Documentation/devicetree/bindings/mfd/cros_ec.txt
> 

That's actually removed you should base your changes on top of

https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git/commit/?h=for-mfd-next&id=46b5780688c0d825b6b8d49b267b13102bea512d


> Series-to: LKML <linux-kernel@vger.kernel.org>
> Series-cc: Raul E Rangel <rrangel@chromium.org>, Furquan Shaikh <furquan@chromium.org>, Duncan Laurie <dlaurie@google.com>, Eric Peers <epeers@google.com>, Benson Leung <bleung@chromium.org>, Enric Balletbo i Serra <enric.balletbo@collabora.com>, Guenter Roeck <groeck@chromium.org>, linux-kernel@vger.kernel.org, Lee Jones <lee.jones@linaro.org>, Rob Herring <robh+dt@kernel.org>, devicetree@vger.kernel.org
> 

I think you need to fix your patman workflow. This should be removed from here.


> Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@chromium.org>
> Change-Id: Icfeab15fa04daaffc61280faf5a75cd9b23ee822

The Change-Id is useless upstream, please remove it.

> Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@google.com>

Only one signed-off per person please.

> ---
>  Documentation/devicetree/bindings/mfd/cros-ec.txt | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/cros-ec.txt b/Documentation/devicetree/bindings/mfd/cros-ec.txt
> index 4860eabd0f729..ec8c5d7ecc266 100644
> --- a/Documentation/devicetree/bindings/mfd/cros-ec.txt
> +++ b/Documentation/devicetree/bindings/mfd/cros-ec.txt
> @@ -3,7 +3,7 @@ ChromeOS Embedded Controller
>  Google's ChromeOS EC is a Cortex-M device which talks to the AP and
>  implements various function such as keyboard and battery charging.
>  
> -The EC can be connect through various means (I2C, SPI, LPC, RPMSG) and the
> +The EC can be connect through various means (I2C, SPI, UART, LPC, RPMSG) and the
>  compatible string used depends on the interface. Each connection method has
>  its own driver which connects to the top level interface-agnostic EC driver.
>  Other Linux driver (such as cros-ec-keyb for the matrix keyboard) connect to
> @@ -17,6 +17,10 @@ Required properties (SPI):
>  - compatible: "google,cros-ec-spi"
>  - reg: SPI chip select
>  
> +Required properties (UART):
> +- compatible: "google,cros-ec-uart"
> +- reg: UART baudrate, flowcontrol
> +

That's odd, a reg that is mean to contain the baudrate and the flowcontrol? How?

>  Required properties (RPMSG):
>  - compatible: "google,cros-ec-rpmsg"
>  
> @@ -72,5 +76,6 @@ spi@131b0000 {
>  	};
>  };
>  
> -
>  Example for LPC is not supplied as it is not yet implemented.
> +
> +Example for UART is not supplied as it is not yet implemented.
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] dt-bindings: mfd: Add DT compatible string "google,cros_ec_uart"
@ 2020-07-15 15:40     ` Enric Balletbo i Serra
  0 siblings, 0 replies; 10+ messages in thread
From: Enric Balletbo i Serra @ 2020-07-15 15:40 UTC (permalink / raw)
  To: Bhanu Prakash Maiya, linux-arm-kernel
  Cc: Rob Herring, Mauro Carvalho Chehab, Duncan Laurie, devicetree,
	Furquan Shaikh, Lee Jones, linux-kernel, Bhanu Prakash Maiya,
	David S . Miller, Raul E Rangel, Greg Kroah-Hartman,
	Guenter Roeck, Benson Leung, Eric Peers

Hi bhanu,

Thank you for your patch. This patch has some style problems, please make sure
to fix and resent the patch.

On 15/7/20 10:25, Bhanu Prakash Maiya wrote:
> From: Bhanu Prakash Maiya <bhanumaiya@chromium.org>
> 
> Add DT compatible string in
> Documentation/devicetree/bindings/mfd/cros_ec.txt
> 

That's actually removed you should base your changes on top of

https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git/commit/?h=for-mfd-next&id=46b5780688c0d825b6b8d49b267b13102bea512d


> Series-to: LKML <linux-kernel@vger.kernel.org>
> Series-cc: Raul E Rangel <rrangel@chromium.org>, Furquan Shaikh <furquan@chromium.org>, Duncan Laurie <dlaurie@google.com>, Eric Peers <epeers@google.com>, Benson Leung <bleung@chromium.org>, Enric Balletbo i Serra <enric.balletbo@collabora.com>, Guenter Roeck <groeck@chromium.org>, linux-kernel@vger.kernel.org, Lee Jones <lee.jones@linaro.org>, Rob Herring <robh+dt@kernel.org>, devicetree@vger.kernel.org
> 

I think you need to fix your patman workflow. This should be removed from here.


> Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@chromium.org>
> Change-Id: Icfeab15fa04daaffc61280faf5a75cd9b23ee822

The Change-Id is useless upstream, please remove it.

> Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@google.com>

Only one signed-off per person please.

> ---
>  Documentation/devicetree/bindings/mfd/cros-ec.txt | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/cros-ec.txt b/Documentation/devicetree/bindings/mfd/cros-ec.txt
> index 4860eabd0f729..ec8c5d7ecc266 100644
> --- a/Documentation/devicetree/bindings/mfd/cros-ec.txt
> +++ b/Documentation/devicetree/bindings/mfd/cros-ec.txt
> @@ -3,7 +3,7 @@ ChromeOS Embedded Controller
>  Google's ChromeOS EC is a Cortex-M device which talks to the AP and
>  implements various function such as keyboard and battery charging.
>  
> -The EC can be connect through various means (I2C, SPI, LPC, RPMSG) and the
> +The EC can be connect through various means (I2C, SPI, UART, LPC, RPMSG) and the
>  compatible string used depends on the interface. Each connection method has
>  its own driver which connects to the top level interface-agnostic EC driver.
>  Other Linux driver (such as cros-ec-keyb for the matrix keyboard) connect to
> @@ -17,6 +17,10 @@ Required properties (SPI):
>  - compatible: "google,cros-ec-spi"
>  - reg: SPI chip select
>  
> +Required properties (UART):
> +- compatible: "google,cros-ec-uart"
> +- reg: UART baudrate, flowcontrol
> +

That's odd, a reg that is mean to contain the baudrate and the flowcontrol? How?

>  Required properties (RPMSG):
>  - compatible: "google,cros-ec-rpmsg"
>  
> @@ -72,5 +76,6 @@ spi@131b0000 {
>  	};
>  };
>  
> -
>  Example for LPC is not supplied as it is not yet implemented.
> +
> +Example for UART is not supplied as it is not yet implemented.
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] cros: platform/chrome: Add cros-ec-uart driver for uart support
  2020-07-15  8:25 ` Bhanu Prakash Maiya
@ 2020-07-15 15:48   ` Enric Balletbo i Serra
  -1 siblings, 0 replies; 10+ messages in thread
From: Enric Balletbo i Serra @ 2020-07-15 15:48 UTC (permalink / raw)
  To: Bhanu Prakash Maiya, linux-arm-kernel
  Cc: Lee Jones, Furquan Shaikh, Raul E Rangel, Eric Peers,
	Duncan Laurie, Benson Leung, Guenter Roeck,
	Mauro Carvalho Chehab, David S . Miller, Rob Herring,
	Greg Kroah-Hartman, devicetree, linux-kernel, kernel test robot,
	Bhanu Prakash Maiya

Hi Bhanu,

Thank you for your patch. This patch has some style problems, please make sure
you fix for next version. Also the subject should be something like
"platform/chrome: cros_ec_uart: Add cros-ec-uart transport layer"

On 15/7/20 10:25, Bhanu Prakash Maiya wrote:
> From: Bhanu Prakash Maiya <bhanumaiya@chromium.org>
> 
> This patch enables uart transport layer for cros_ec framework.
> The cros-ec-uart binds with EC device working on uart transport to
> send request and receive response.
> 
> Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@chromium.org>

Fix your email, use your chromium or your google address but not both.

> Reported-by: kernel test robot <lkp@intel.com>

I assume a  build error was reported by lkp on your first version?

The Reported-by tag is a bit weird here because the historic of what is reported
was lost. You can remove the tag, or if you want to maintain the historic please
add and explanation like this

[Fixed a build issue due ... ]
Reported-by: kernel test robot <lkp@intel.com>

> Change-Id: Icb23b633700f1ef4d123e3f21fd26fad21a3f207

The Change-Id has no meaning upstream, so remove it.

> Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@google.com>

Only one signed-off per person, please.

> ---
> Changes in v2:
> 1: Fixed build error on v1.
> 2: Changed EC timeout for response packet to 3 Sec and added comments.
> 3: Fixed cros_ec_uart_rx_bytes function to handle rx buffer < size of response header.
> 
>  MAINTAINERS                            |   1 +
>  drivers/platform/chrome/Kconfig        |  10 +
>  drivers/platform/chrome/Makefile       |   1 +
>  drivers/platform/chrome/cros_ec_uart.c | 411 +++++++++++++++++++++++++
>  4 files changed, 423 insertions(+)
>  create mode 100644 drivers/platform/chrome/cros_ec_uart.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 50659d76976b7..aa8e1d121c1d7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4036,6 +4036,7 @@ F:	sound/soc/codecs/cros_ec_codec.*
>  CHROMEOS EC SUBDRIVERS
>  M:	Benson Leung <bleung@chromium.org>
>  M:	Enric Balletbo i Serra <enric.balletbo@collabora.com>
> +M:	Bhanu Prakash Maiya <bhanumaiya@chromium.org>

That's for those unmaintained drivers, what you want probably is be the
maintainer of cros_ec_uart so you should add a new entry like this

CHROMEOS EC UART DRIVER
M:	Bhanu Prakash Maiya <bhanumaiya@chromium.org>
R:	Enric Balletbo i Serra <enric.balletbo@collabora.com>
S:	Maintained
F:	drivers/platform/chrome/cros_ec_uart.c

(Please, add me as a reviewer too)

>  R:	Guenter Roeck <groeck@chromium.org>
>  S:	Maintained
>  F:	drivers/power/supply/cros_usbpd-charger.c
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index 3822e5e111caa..2082fafe08a6a 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -125,6 +125,16 @@ config CROS_EC_SPI
>  	  response time cannot be guaranteed, we support ignoring
>  	  'pre-amble' bytes before the response actually starts.
>  
> +config CROS_EC_UART
> +	tristate "ChromeOS Embedded Controller (UART)"
> +	depends on CROS_EC && ACPI && SERIAL_DEV_BUS
> +	help
> +	  If you say Y here, you get support for talking to the ChromeOS EC
> +	  through a UART, using a byte-level protocol.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called cros_ec_uart.
> +
>  config CROS_EC_LPC
>  	tristate "ChromeOS Embedded Controller (LPC)"
>  	depends on CROS_EC && ACPI && (X86 || COMPILE_TEST)
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index 8ed1e33033b38..fc449351fc794 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_CROS_EC_I2C)		+= cros_ec_i2c.o
>  obj-$(CONFIG_CROS_EC_ISHTP)		+= cros_ec_ishtp.o
>  obj-$(CONFIG_CROS_EC_RPMSG)		+= cros_ec_rpmsg.o
>  obj-$(CONFIG_CROS_EC_SPI)		+= cros_ec_spi.o
> +obj-$(CONFIG_CROS_EC_UART)		+= cros_ec_uart.o
>  cros_ec_lpcs-objs			:= cros_ec_lpc.o cros_ec_lpc_mec.o
>  obj-$(CONFIG_CROS_EC_TYPEC)		+= cros_ec_typec.o
>  obj-$(CONFIG_CROS_EC_LPC)		+= cros_ec_lpcs.o
> diff --git a/drivers/platform/chrome/cros_ec_uart.c b/drivers/platform/chrome/cros_ec_uart.c
> new file mode 100644
> index 0000000000000..9a0ba884b6812
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_ec_uart.c
> @@ -0,0 +1,411 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * UART interface for ChromeOS Embedded Controller
> + *
> + * Copyright 2020 Google LLC.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/acpi.h>
> +#include <linux/of.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
> +#include <linux/serdev.h>
> +#include <linux/slab.h>
> +#include <uapi/linux/sched/types.h>
> +
> +#include "cros_ec.h"
> +
> +/*
> + * EC sends contiguous bytes of response packet on UART AP RX.
> + * TTY driver in AP accumulates incoming bytes and calls the registered callback
> + * function. Byte count can range from 1 to Max count supported by TTY driver.
> + * This driver should wait for long time for all callbacks to be processed.
> + * Considering the worst case scenario, wait for ~3 secs. This timeout should
> + * account for max latency and some additional guard time.
> + * In case the packet is received in ms, wait queue will be released and packet
> + * will be processed.
> + */
> +#define EC_MSG_DEADLINE_MS		(300 * 10)
> +
> +/**
> + * struct response_info - Encapsulate EC response related
> + *			information for passing between function
> + *			cros_ec_uart_pkt_xfer() and cros_ec_uart_rx_bytes()
> + *			callback.
> + * @data:		Copy the data received from EC here.
> + * @max_size:		Max size allocated for the @data buffer. If the
> + *			received data exceeds this value, we log an error.
> + * @size:		Actual size of data received from EC. This is also
> + *			used to accumulate byte count with response is received
> + *			in dma chunks.
> + * @exp_len:		Expected bytes of response from EC including header.
> + * @error:		0 for success, negative error code for a failure.
> + * @received:		Set to true on receiving a valid EC response.
> + * @wait_queue:		Wait queue EC response where the cros_ec sends request
> + *			to EC and waits
> + */
> +struct response_info {
> +	void *data;
> +	size_t max_size;
> +	size_t size;
> +	int error;
> +	size_t exp_len;
> +	bool received;
> +	wait_queue_head_t wait_queue;
> +};
> +
> +/**
> + * struct cros_ec_uart - information about a uart-connected EC
> + *
> + * @serdev_device:	serdev uart device we are connected to.
> + * @baudrate:		UART baudrate of attached EC device.
> + * @flowcontrol:	UART flowcontrol of attached device.
> + * @irq:		Linux IRQ number of associated serial device.
> + * @response:		Response info passing between cros_ec_uart_pkt_xfer()
> + *			and cros_ec_uart_rx_bytes()
> + */
> +struct cros_ec_uart {
> +	struct serdev_device *serdev;
> +	u32 baudrate;
> +	u8  flowcontrol;
> +	u32 irq;
> +	struct response_info response;
> +};
> +
> +static int cros_ec_uart_rx_bytes(struct serdev_device *serdev,
> +				 const u8 *data,
> +				 size_t count)
> +{
> +	struct ec_host_response *response;
> +	struct cros_ec_device *ec_dev =   serdev_device_get_drvdata(serdev);

Use only one space around =

> +	struct cros_ec_uart *ec_uart = ec_dev->priv;
> +
> +	/* Check if bytes were sent out of band */
> +	if (!ec_uart->response.data) {

No need for { } around here

> +		/* Discard all bytes */
> +		return count;
> +	}
> +
> +	/*
> +	 * Check if incoming bytes + response.size are less than allocated
> +	 * buffer in din by cros_ec. This will ensure that if EC sends more
> +	 * bytes than max_size, waiting process will be notified with an error.
> +	 */
> +	if (ec_uart->response.size + count <= ec_uart->response.max_size) {
> +		/* Copy bytes in data in buffer */
> +		memcpy((void *)ec_uart->response.data + ec_uart->response.size,
> +		       (void *)data, count);
> +
> +		/* Add incoming bytes in size */
> +		ec_uart->response.size += count;
> +
> +		/*
> +		 * Read data_len if we received response header and if exp_len
> +		 * was not read before.
> +		 */
> +		if (ec_uart->response.size >= sizeof(*response) &&
> +		    ec_uart->response.exp_len == 0) {
> +			/* Get expected response length from response header */
> +			response = (struct ec_host_response *)
> +							ec_uart->response.data;

Given the new relaxed view on the line length as long as it's below 100 and
helps readability, put the above on one line. This comment applies on all those
cases that a line below 100 is possible.

> +
> +			ec_uart->response.exp_len = response->data_len +
> +				sizeof(*response);
> +		}
> +
> +		/*
> +		 * If driver received response header and payload from EC,
> +		 * Wake up the wait queue.
> +		 */
> +		if (ec_uart->response.size >= sizeof(*response) &&
> +		    ec_uart->response.size == ec_uart->response.exp_len) {
> +			/* Set flag before waking up the caller */
> +			ec_uart->response.received = true;
> +
> +			/* Wake the calling thread */
> +			wake_up_interruptible(&ec_uart->response.wait_queue);
> +		}
> +	} else {
> +		/* Received bytes are more the allocated buffer*/
> +		ec_uart->response.error = -EMSGSIZE;
> +
> +		/* Wake the calling thread */
> +		wake_up_interruptible(&ec_uart->response.wait_queue);
> +	}
> +
> +	return count;
> +}
> +
> +static int cros_ec_uart_pkt_xfer(struct cros_ec_device *ec_dev,
> +				 struct cros_ec_command *ec_msg)
> +{
> +	struct cros_ec_uart *ec_uart = ec_dev->priv;
> +	struct serdev_device *serdev = ec_uart->serdev;
> +	struct ec_host_response *response;
> +	unsigned int len;
> +	int ret, i;
> +	u8 sum = 0;
> +
> +	/* Prepare an outgoing message in the output buffer */
> +	len = cros_ec_prepare_tx(ec_dev, ec_msg);
> +	dev_dbg(ec_dev->dev, "Prepared len=%d\n", len);
> +
> +	/* Setup for incoming response */
> +	ec_uart->response.data = ec_dev->din;
> +	ec_uart->response.max_size = ec_dev->din_size;
> +	ec_uart->response.size = 0;
> +	ec_uart->response.error = 0;
> +	ec_uart->response.exp_len = 0;
> +	ec_uart->response.received = false;
> +
> +	/* Write serial device buffer */
> +	ret = serdev_device_write_buf(serdev, ec_dev->dout, len);
> +	if (ret < len) {
> +		dev_err(&serdev->dev,
> +			"Unable to write data to serial device %s",
> +			dev_name(&serdev->dev));
> +
> +		/* Return EIO as controller had issues writing buffer */
> +		ret = -EIO;
> +		goto exit;
> +	}
> +
> +	/* Once request is successfully sent to EC, wait to wait_queue */
> +	wait_event_interruptible_timeout(ec_uart->response.wait_queue,
> +					 ec_uart->response.received,
> +					 msecs_to_jiffies(EC_MSG_DEADLINE_MS));
> +
> +	/* Check if wait_queue was interrupted due to an error */
> +	if (ec_uart->response.error < 0) {
> +		dev_warn(&serdev->dev, "Response error detected.\n");
> +
> +		ret = ec_uart->response.error;
> +		goto exit;
> +	}
> +
> +	/* Check if valid response was received or there was a timeout */
> +	if (!ec_uart->response.received) {
> +		dev_warn(&serdev->dev, "EC failed to respond in time.\n");
> +
> +		ret = -ETIMEDOUT;
> +		goto exit;
> +	}
> +
> +	/* Check response error code */
> +	response = (struct ec_host_response *)ec_dev->din;
> +	ec_msg->result = response->result;
> +
> +	/* Check if received response is longer than expected */
> +	if (response->data_len > ec_msg->insize) {
> +		dev_err(ec_dev->dev, "Resp too long (%d bytes, expected %d)",
> +			response->data_len,
> +			ec_msg->insize);
> +		ret = -ENOSPC;
> +		goto exit;
> +	}
> +
> +	/* Copy response packet to ec_msg data buffer */
> +	memcpy(ec_msg->data,
> +	       ec_dev->din + sizeof(*response),
> +	       response->data_len);
> +
> +	/* Add all response header bytes for checksum calculation */
> +	for (i = 0; i < sizeof(*response); i++)
> +		sum += ec_dev->din[i];
> +
> +	/* Copy response packet payload and compute checksum */
> +	for (i = 0; i < response->data_len; i++)
> +		sum += ec_msg->data[i];
> +
> +	if (sum) {
> +		dev_err(ec_dev->dev,
> +			"Bad packet checksum calculated %x\n",
> +			sum);
> +		ret = -EBADMSG;
> +		goto exit;
> +	}
> +
> +	/* Return data_len to cros_ec */
> +	ret = response->data_len;
> +
> +exit:
> +	/* Reset ec_uart */
> +	ec_uart->response.data = NULL;
> +	ec_uart->response.max_size = 0;
> +	ec_uart->response.size = 0;
> +	ec_uart->response.error = 0;
> +	ec_uart->response.exp_len = 0;
> +	ec_uart->response.received = false;
> +
> +	if (ec_msg->command == EC_CMD_REBOOT_EC)
> +		msleep(EC_REBOOT_DELAY_MS);
> +
> +	return ret;
> +}
> +
> +static int cros_ec_uart_resource(struct acpi_resource *ares, void *data)
> +{
> +	struct cros_ec_uart *ec_uart = data;
> +	struct acpi_resource_uart_serialbus *sb;
> +
> +	switch (ares->type) {
> +	case ACPI_RESOURCE_TYPE_SERIAL_BUS:
> +		sb = &ares->data.uart_serial_bus;
> +		if (sb->type == ACPI_RESOURCE_SERIAL_TYPE_UART) {
> +			ec_uart->baudrate = sb->default_baud_rate;
> +			dev_dbg(&ec_uart->serdev->dev, "Baudrate %d\n",
> +				ec_uart->baudrate);
> +
> +			ec_uart->flowcontrol = sb->flow_control;
> +			dev_dbg(&ec_uart->serdev->dev, "Flow control %d\n",
> +				ec_uart->flowcontrol);
> +		}
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static int cros_ec_uart_acpi_probe(struct cros_ec_uart *ec_uart)
> +{
> +	LIST_HEAD(resources);
> +	struct acpi_device *adev = ACPI_COMPANION(&ec_uart->serdev->dev);
> +	int ret;
> +
> +	/* Retrieve UART ACPI info */
> +	ret = acpi_dev_get_resources(adev, &resources,
> +				     cros_ec_uart_resource, ec_uart);
> +	if (ret < 0)
> +		return ret;
> +
> +	acpi_dev_free_resource_list(&resources);
> +
> +	/* Retrieve GpioInt and translate it to Linux IRQ number */
> +	ret = acpi_dev_gpio_irq_get(adev, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	ec_uart->irq = ret;
> +	dev_dbg(&ec_uart->serdev->dev, "IRQ number %d\n", ec_uart->irq);
> +
> +	return 0;
> +}
> +
> +static const struct serdev_device_ops cros_ec_uart_client_ops = {
> +	.receive_buf = cros_ec_uart_rx_bytes,
> +};
> +
> +static int cros_ec_uart_probe(struct serdev_device *serdev)
> +{
> +	struct device *dev = &serdev->dev;
> +	struct cros_ec_device *ec_dev;
> +	struct cros_ec_uart *ec_uart;
> +	int ret;
> +
> +	ec_uart = devm_kzalloc(dev, sizeof(*ec_uart), GFP_KERNEL);
> +	if (!ec_uart)
> +		return -ENOMEM;
> +
> +	ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL);
> +	if (!ec_dev)
> +		return -ENOMEM;
> +
> +	ec_uart->serdev = serdev;
> +
> +	/* Open the serial device */
> +	ret = devm_serdev_device_open(dev, ec_uart->serdev);
> +	if (ret) {
> +		dev_err(dev, "Unable to open UART device %s",
> +			dev_name(&serdev->dev));
> +		return ret;
> +	}
> +
> +	serdev_device_set_drvdata(serdev, ec_dev);
> +
> +	serdev_device_set_client_ops(serdev, &cros_ec_uart_client_ops);
> +
> +	/* Initialize wait queue */
> +	init_waitqueue_head(&ec_uart->response.wait_queue);
> +
> +	ret = cros_ec_uart_acpi_probe(ec_uart);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to get ACPI info (%d)", ret);
> +		return ret;
> +	}
> +
> +	/* Set baud rate of serial device */
> +	ret = serdev_device_set_baudrate(serdev, ec_uart->baudrate);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to set up host baud rate (%d)", ret);
> +		return ret;
> +	}
> +
> +	/* Set flow control of serial device */
> +	serdev_device_set_flow_control(serdev, ec_uart->flowcontrol);
> +
> +	/* Initialize ec_dev for cros_ec  */
> +	ec_dev->phys_name = dev_name(&ec_uart->serdev->dev);
> +	ec_dev->dev = dev;
> +	ec_dev->priv = ec_uart;
> +	ec_dev->irq = ec_uart->irq;
> +	ec_dev->cmd_xfer = NULL;
> +	ec_dev->pkt_xfer = cros_ec_uart_pkt_xfer;
> +	ec_dev->din_size = sizeof(struct ec_host_response) +
> +			   sizeof(struct ec_response_get_protocol_info);
> +	ec_dev->dout_size = sizeof(struct ec_host_request);
> +
> +	/* Register a new cros_ec device */
> +	return cros_ec_register(ec_dev);
> +}
> +
> +static void cros_ec_uart_remove(struct serdev_device *serdev)
> +{
> +	struct cros_ec_device *ec_dev = serdev_device_get_drvdata(serdev);
> +
> +	cros_ec_unregister(ec_dev);
> +};
> +
> +static int __maybe_unused cros_ec_uart_suspend(struct device *dev)
> +{
> +	struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
> +
> +	return cros_ec_suspend(ec_dev);
> +}
> +
> +static int __maybe_unused cros_ec_uart_resume(struct device *dev)
> +{
> +	struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
> +
> +	return cros_ec_resume(ec_dev);
> +}
> +
> +static SIMPLE_DEV_PM_OPS(cros_ec_uart_pm_ops, cros_ec_uart_suspend,
> +			 cros_ec_uart_resume);
> +
> +static const struct of_device_id cros_ec_uart_of_match[] = {
> +	{ .compatible = "google,cros-ec-uart" },

So this is an ACPI driver that is instantiated by using the Device Tree
namespace link in ACPI. I am not sure how to deal with this, but I suspect we
need a proper OF binding definition before accept it, ideally, could match with
the ACPI definition and use the fwnode_property API to get the properties?

Rob, any thought on this?

> +	{}
> +};
> +
> +static struct serdev_device_driver cros_ec_uart_driver = {
> +	.driver	= {
> +		.name	= "cros-ec-uart",
> +		.of_match_table = cros_ec_uart_of_match,
> +		.pm	= &cros_ec_uart_pm_ops,
> +	},
> +	.probe		= cros_ec_uart_probe,
> +	.remove		= cros_ec_uart_remove,
> +};
> +
> +module_serdev_device_driver(cros_ec_uart_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("UART interface for ChromeOS Embedded Controller");
> +MODULE_AUTHOR("Bhanu Prakash Maiya <bhanumaiya@chromium.org>");
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] cros: platform/chrome: Add cros-ec-uart driver for uart support
@ 2020-07-15 15:48   ` Enric Balletbo i Serra
  0 siblings, 0 replies; 10+ messages in thread
From: Enric Balletbo i Serra @ 2020-07-15 15:48 UTC (permalink / raw)
  To: Bhanu Prakash Maiya, linux-arm-kernel
  Cc: Rob Herring, Mauro Carvalho Chehab, Duncan Laurie,
	kernel test robot, devicetree, Furquan Shaikh, Lee Jones,
	linux-kernel, Bhanu Prakash Maiya, David S . Miller,
	Raul E Rangel, Greg Kroah-Hartman, Guenter Roeck, Benson Leung,
	Eric Peers

Hi Bhanu,

Thank you for your patch. This patch has some style problems, please make sure
you fix for next version. Also the subject should be something like
"platform/chrome: cros_ec_uart: Add cros-ec-uart transport layer"

On 15/7/20 10:25, Bhanu Prakash Maiya wrote:
> From: Bhanu Prakash Maiya <bhanumaiya@chromium.org>
> 
> This patch enables uart transport layer for cros_ec framework.
> The cros-ec-uart binds with EC device working on uart transport to
> send request and receive response.
> 
> Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@chromium.org>

Fix your email, use your chromium or your google address but not both.

> Reported-by: kernel test robot <lkp@intel.com>

I assume a  build error was reported by lkp on your first version?

The Reported-by tag is a bit weird here because the historic of what is reported
was lost. You can remove the tag, or if you want to maintain the historic please
add and explanation like this

[Fixed a build issue due ... ]
Reported-by: kernel test robot <lkp@intel.com>

> Change-Id: Icb23b633700f1ef4d123e3f21fd26fad21a3f207

The Change-Id has no meaning upstream, so remove it.

> Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@google.com>

Only one signed-off per person, please.

> ---
> Changes in v2:
> 1: Fixed build error on v1.
> 2: Changed EC timeout for response packet to 3 Sec and added comments.
> 3: Fixed cros_ec_uart_rx_bytes function to handle rx buffer < size of response header.
> 
>  MAINTAINERS                            |   1 +
>  drivers/platform/chrome/Kconfig        |  10 +
>  drivers/platform/chrome/Makefile       |   1 +
>  drivers/platform/chrome/cros_ec_uart.c | 411 +++++++++++++++++++++++++
>  4 files changed, 423 insertions(+)
>  create mode 100644 drivers/platform/chrome/cros_ec_uart.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 50659d76976b7..aa8e1d121c1d7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4036,6 +4036,7 @@ F:	sound/soc/codecs/cros_ec_codec.*
>  CHROMEOS EC SUBDRIVERS
>  M:	Benson Leung <bleung@chromium.org>
>  M:	Enric Balletbo i Serra <enric.balletbo@collabora.com>
> +M:	Bhanu Prakash Maiya <bhanumaiya@chromium.org>

That's for those unmaintained drivers, what you want probably is be the
maintainer of cros_ec_uart so you should add a new entry like this

CHROMEOS EC UART DRIVER
M:	Bhanu Prakash Maiya <bhanumaiya@chromium.org>
R:	Enric Balletbo i Serra <enric.balletbo@collabora.com>
S:	Maintained
F:	drivers/platform/chrome/cros_ec_uart.c

(Please, add me as a reviewer too)

>  R:	Guenter Roeck <groeck@chromium.org>
>  S:	Maintained
>  F:	drivers/power/supply/cros_usbpd-charger.c
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index 3822e5e111caa..2082fafe08a6a 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -125,6 +125,16 @@ config CROS_EC_SPI
>  	  response time cannot be guaranteed, we support ignoring
>  	  'pre-amble' bytes before the response actually starts.
>  
> +config CROS_EC_UART
> +	tristate "ChromeOS Embedded Controller (UART)"
> +	depends on CROS_EC && ACPI && SERIAL_DEV_BUS
> +	help
> +	  If you say Y here, you get support for talking to the ChromeOS EC
> +	  through a UART, using a byte-level protocol.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called cros_ec_uart.
> +
>  config CROS_EC_LPC
>  	tristate "ChromeOS Embedded Controller (LPC)"
>  	depends on CROS_EC && ACPI && (X86 || COMPILE_TEST)
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index 8ed1e33033b38..fc449351fc794 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_CROS_EC_I2C)		+= cros_ec_i2c.o
>  obj-$(CONFIG_CROS_EC_ISHTP)		+= cros_ec_ishtp.o
>  obj-$(CONFIG_CROS_EC_RPMSG)		+= cros_ec_rpmsg.o
>  obj-$(CONFIG_CROS_EC_SPI)		+= cros_ec_spi.o
> +obj-$(CONFIG_CROS_EC_UART)		+= cros_ec_uart.o
>  cros_ec_lpcs-objs			:= cros_ec_lpc.o cros_ec_lpc_mec.o
>  obj-$(CONFIG_CROS_EC_TYPEC)		+= cros_ec_typec.o
>  obj-$(CONFIG_CROS_EC_LPC)		+= cros_ec_lpcs.o
> diff --git a/drivers/platform/chrome/cros_ec_uart.c b/drivers/platform/chrome/cros_ec_uart.c
> new file mode 100644
> index 0000000000000..9a0ba884b6812
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_ec_uart.c
> @@ -0,0 +1,411 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * UART interface for ChromeOS Embedded Controller
> + *
> + * Copyright 2020 Google LLC.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/acpi.h>
> +#include <linux/of.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
> +#include <linux/serdev.h>
> +#include <linux/slab.h>
> +#include <uapi/linux/sched/types.h>
> +
> +#include "cros_ec.h"
> +
> +/*
> + * EC sends contiguous bytes of response packet on UART AP RX.
> + * TTY driver in AP accumulates incoming bytes and calls the registered callback
> + * function. Byte count can range from 1 to Max count supported by TTY driver.
> + * This driver should wait for long time for all callbacks to be processed.
> + * Considering the worst case scenario, wait for ~3 secs. This timeout should
> + * account for max latency and some additional guard time.
> + * In case the packet is received in ms, wait queue will be released and packet
> + * will be processed.
> + */
> +#define EC_MSG_DEADLINE_MS		(300 * 10)
> +
> +/**
> + * struct response_info - Encapsulate EC response related
> + *			information for passing between function
> + *			cros_ec_uart_pkt_xfer() and cros_ec_uart_rx_bytes()
> + *			callback.
> + * @data:		Copy the data received from EC here.
> + * @max_size:		Max size allocated for the @data buffer. If the
> + *			received data exceeds this value, we log an error.
> + * @size:		Actual size of data received from EC. This is also
> + *			used to accumulate byte count with response is received
> + *			in dma chunks.
> + * @exp_len:		Expected bytes of response from EC including header.
> + * @error:		0 for success, negative error code for a failure.
> + * @received:		Set to true on receiving a valid EC response.
> + * @wait_queue:		Wait queue EC response where the cros_ec sends request
> + *			to EC and waits
> + */
> +struct response_info {
> +	void *data;
> +	size_t max_size;
> +	size_t size;
> +	int error;
> +	size_t exp_len;
> +	bool received;
> +	wait_queue_head_t wait_queue;
> +};
> +
> +/**
> + * struct cros_ec_uart - information about a uart-connected EC
> + *
> + * @serdev_device:	serdev uart device we are connected to.
> + * @baudrate:		UART baudrate of attached EC device.
> + * @flowcontrol:	UART flowcontrol of attached device.
> + * @irq:		Linux IRQ number of associated serial device.
> + * @response:		Response info passing between cros_ec_uart_pkt_xfer()
> + *			and cros_ec_uart_rx_bytes()
> + */
> +struct cros_ec_uart {
> +	struct serdev_device *serdev;
> +	u32 baudrate;
> +	u8  flowcontrol;
> +	u32 irq;
> +	struct response_info response;
> +};
> +
> +static int cros_ec_uart_rx_bytes(struct serdev_device *serdev,
> +				 const u8 *data,
> +				 size_t count)
> +{
> +	struct ec_host_response *response;
> +	struct cros_ec_device *ec_dev =   serdev_device_get_drvdata(serdev);

Use only one space around =

> +	struct cros_ec_uart *ec_uart = ec_dev->priv;
> +
> +	/* Check if bytes were sent out of band */
> +	if (!ec_uart->response.data) {

No need for { } around here

> +		/* Discard all bytes */
> +		return count;
> +	}
> +
> +	/*
> +	 * Check if incoming bytes + response.size are less than allocated
> +	 * buffer in din by cros_ec. This will ensure that if EC sends more
> +	 * bytes than max_size, waiting process will be notified with an error.
> +	 */
> +	if (ec_uart->response.size + count <= ec_uart->response.max_size) {
> +		/* Copy bytes in data in buffer */
> +		memcpy((void *)ec_uart->response.data + ec_uart->response.size,
> +		       (void *)data, count);
> +
> +		/* Add incoming bytes in size */
> +		ec_uart->response.size += count;
> +
> +		/*
> +		 * Read data_len if we received response header and if exp_len
> +		 * was not read before.
> +		 */
> +		if (ec_uart->response.size >= sizeof(*response) &&
> +		    ec_uart->response.exp_len == 0) {
> +			/* Get expected response length from response header */
> +			response = (struct ec_host_response *)
> +							ec_uart->response.data;

Given the new relaxed view on the line length as long as it's below 100 and
helps readability, put the above on one line. This comment applies on all those
cases that a line below 100 is possible.

> +
> +			ec_uart->response.exp_len = response->data_len +
> +				sizeof(*response);
> +		}
> +
> +		/*
> +		 * If driver received response header and payload from EC,
> +		 * Wake up the wait queue.
> +		 */
> +		if (ec_uart->response.size >= sizeof(*response) &&
> +		    ec_uart->response.size == ec_uart->response.exp_len) {
> +			/* Set flag before waking up the caller */
> +			ec_uart->response.received = true;
> +
> +			/* Wake the calling thread */
> +			wake_up_interruptible(&ec_uart->response.wait_queue);
> +		}
> +	} else {
> +		/* Received bytes are more the allocated buffer*/
> +		ec_uart->response.error = -EMSGSIZE;
> +
> +		/* Wake the calling thread */
> +		wake_up_interruptible(&ec_uart->response.wait_queue);
> +	}
> +
> +	return count;
> +}
> +
> +static int cros_ec_uart_pkt_xfer(struct cros_ec_device *ec_dev,
> +				 struct cros_ec_command *ec_msg)
> +{
> +	struct cros_ec_uart *ec_uart = ec_dev->priv;
> +	struct serdev_device *serdev = ec_uart->serdev;
> +	struct ec_host_response *response;
> +	unsigned int len;
> +	int ret, i;
> +	u8 sum = 0;
> +
> +	/* Prepare an outgoing message in the output buffer */
> +	len = cros_ec_prepare_tx(ec_dev, ec_msg);
> +	dev_dbg(ec_dev->dev, "Prepared len=%d\n", len);
> +
> +	/* Setup for incoming response */
> +	ec_uart->response.data = ec_dev->din;
> +	ec_uart->response.max_size = ec_dev->din_size;
> +	ec_uart->response.size = 0;
> +	ec_uart->response.error = 0;
> +	ec_uart->response.exp_len = 0;
> +	ec_uart->response.received = false;
> +
> +	/* Write serial device buffer */
> +	ret = serdev_device_write_buf(serdev, ec_dev->dout, len);
> +	if (ret < len) {
> +		dev_err(&serdev->dev,
> +			"Unable to write data to serial device %s",
> +			dev_name(&serdev->dev));
> +
> +		/* Return EIO as controller had issues writing buffer */
> +		ret = -EIO;
> +		goto exit;
> +	}
> +
> +	/* Once request is successfully sent to EC, wait to wait_queue */
> +	wait_event_interruptible_timeout(ec_uart->response.wait_queue,
> +					 ec_uart->response.received,
> +					 msecs_to_jiffies(EC_MSG_DEADLINE_MS));
> +
> +	/* Check if wait_queue was interrupted due to an error */
> +	if (ec_uart->response.error < 0) {
> +		dev_warn(&serdev->dev, "Response error detected.\n");
> +
> +		ret = ec_uart->response.error;
> +		goto exit;
> +	}
> +
> +	/* Check if valid response was received or there was a timeout */
> +	if (!ec_uart->response.received) {
> +		dev_warn(&serdev->dev, "EC failed to respond in time.\n");
> +
> +		ret = -ETIMEDOUT;
> +		goto exit;
> +	}
> +
> +	/* Check response error code */
> +	response = (struct ec_host_response *)ec_dev->din;
> +	ec_msg->result = response->result;
> +
> +	/* Check if received response is longer than expected */
> +	if (response->data_len > ec_msg->insize) {
> +		dev_err(ec_dev->dev, "Resp too long (%d bytes, expected %d)",
> +			response->data_len,
> +			ec_msg->insize);
> +		ret = -ENOSPC;
> +		goto exit;
> +	}
> +
> +	/* Copy response packet to ec_msg data buffer */
> +	memcpy(ec_msg->data,
> +	       ec_dev->din + sizeof(*response),
> +	       response->data_len);
> +
> +	/* Add all response header bytes for checksum calculation */
> +	for (i = 0; i < sizeof(*response); i++)
> +		sum += ec_dev->din[i];
> +
> +	/* Copy response packet payload and compute checksum */
> +	for (i = 0; i < response->data_len; i++)
> +		sum += ec_msg->data[i];
> +
> +	if (sum) {
> +		dev_err(ec_dev->dev,
> +			"Bad packet checksum calculated %x\n",
> +			sum);
> +		ret = -EBADMSG;
> +		goto exit;
> +	}
> +
> +	/* Return data_len to cros_ec */
> +	ret = response->data_len;
> +
> +exit:
> +	/* Reset ec_uart */
> +	ec_uart->response.data = NULL;
> +	ec_uart->response.max_size = 0;
> +	ec_uart->response.size = 0;
> +	ec_uart->response.error = 0;
> +	ec_uart->response.exp_len = 0;
> +	ec_uart->response.received = false;
> +
> +	if (ec_msg->command == EC_CMD_REBOOT_EC)
> +		msleep(EC_REBOOT_DELAY_MS);
> +
> +	return ret;
> +}
> +
> +static int cros_ec_uart_resource(struct acpi_resource *ares, void *data)
> +{
> +	struct cros_ec_uart *ec_uart = data;
> +	struct acpi_resource_uart_serialbus *sb;
> +
> +	switch (ares->type) {
> +	case ACPI_RESOURCE_TYPE_SERIAL_BUS:
> +		sb = &ares->data.uart_serial_bus;
> +		if (sb->type == ACPI_RESOURCE_SERIAL_TYPE_UART) {
> +			ec_uart->baudrate = sb->default_baud_rate;
> +			dev_dbg(&ec_uart->serdev->dev, "Baudrate %d\n",
> +				ec_uart->baudrate);
> +
> +			ec_uart->flowcontrol = sb->flow_control;
> +			dev_dbg(&ec_uart->serdev->dev, "Flow control %d\n",
> +				ec_uart->flowcontrol);
> +		}
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static int cros_ec_uart_acpi_probe(struct cros_ec_uart *ec_uart)
> +{
> +	LIST_HEAD(resources);
> +	struct acpi_device *adev = ACPI_COMPANION(&ec_uart->serdev->dev);
> +	int ret;
> +
> +	/* Retrieve UART ACPI info */
> +	ret = acpi_dev_get_resources(adev, &resources,
> +				     cros_ec_uart_resource, ec_uart);
> +	if (ret < 0)
> +		return ret;
> +
> +	acpi_dev_free_resource_list(&resources);
> +
> +	/* Retrieve GpioInt and translate it to Linux IRQ number */
> +	ret = acpi_dev_gpio_irq_get(adev, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	ec_uart->irq = ret;
> +	dev_dbg(&ec_uart->serdev->dev, "IRQ number %d\n", ec_uart->irq);
> +
> +	return 0;
> +}
> +
> +static const struct serdev_device_ops cros_ec_uart_client_ops = {
> +	.receive_buf = cros_ec_uart_rx_bytes,
> +};
> +
> +static int cros_ec_uart_probe(struct serdev_device *serdev)
> +{
> +	struct device *dev = &serdev->dev;
> +	struct cros_ec_device *ec_dev;
> +	struct cros_ec_uart *ec_uart;
> +	int ret;
> +
> +	ec_uart = devm_kzalloc(dev, sizeof(*ec_uart), GFP_KERNEL);
> +	if (!ec_uart)
> +		return -ENOMEM;
> +
> +	ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL);
> +	if (!ec_dev)
> +		return -ENOMEM;
> +
> +	ec_uart->serdev = serdev;
> +
> +	/* Open the serial device */
> +	ret = devm_serdev_device_open(dev, ec_uart->serdev);
> +	if (ret) {
> +		dev_err(dev, "Unable to open UART device %s",
> +			dev_name(&serdev->dev));
> +		return ret;
> +	}
> +
> +	serdev_device_set_drvdata(serdev, ec_dev);
> +
> +	serdev_device_set_client_ops(serdev, &cros_ec_uart_client_ops);
> +
> +	/* Initialize wait queue */
> +	init_waitqueue_head(&ec_uart->response.wait_queue);
> +
> +	ret = cros_ec_uart_acpi_probe(ec_uart);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to get ACPI info (%d)", ret);
> +		return ret;
> +	}
> +
> +	/* Set baud rate of serial device */
> +	ret = serdev_device_set_baudrate(serdev, ec_uart->baudrate);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to set up host baud rate (%d)", ret);
> +		return ret;
> +	}
> +
> +	/* Set flow control of serial device */
> +	serdev_device_set_flow_control(serdev, ec_uart->flowcontrol);
> +
> +	/* Initialize ec_dev for cros_ec  */
> +	ec_dev->phys_name = dev_name(&ec_uart->serdev->dev);
> +	ec_dev->dev = dev;
> +	ec_dev->priv = ec_uart;
> +	ec_dev->irq = ec_uart->irq;
> +	ec_dev->cmd_xfer = NULL;
> +	ec_dev->pkt_xfer = cros_ec_uart_pkt_xfer;
> +	ec_dev->din_size = sizeof(struct ec_host_response) +
> +			   sizeof(struct ec_response_get_protocol_info);
> +	ec_dev->dout_size = sizeof(struct ec_host_request);
> +
> +	/* Register a new cros_ec device */
> +	return cros_ec_register(ec_dev);
> +}
> +
> +static void cros_ec_uart_remove(struct serdev_device *serdev)
> +{
> +	struct cros_ec_device *ec_dev = serdev_device_get_drvdata(serdev);
> +
> +	cros_ec_unregister(ec_dev);
> +};
> +
> +static int __maybe_unused cros_ec_uart_suspend(struct device *dev)
> +{
> +	struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
> +
> +	return cros_ec_suspend(ec_dev);
> +}
> +
> +static int __maybe_unused cros_ec_uart_resume(struct device *dev)
> +{
> +	struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
> +
> +	return cros_ec_resume(ec_dev);
> +}
> +
> +static SIMPLE_DEV_PM_OPS(cros_ec_uart_pm_ops, cros_ec_uart_suspend,
> +			 cros_ec_uart_resume);
> +
> +static const struct of_device_id cros_ec_uart_of_match[] = {
> +	{ .compatible = "google,cros-ec-uart" },

So this is an ACPI driver that is instantiated by using the Device Tree
namespace link in ACPI. I am not sure how to deal with this, but I suspect we
need a proper OF binding definition before accept it, ideally, could match with
the ACPI definition and use the fwnode_property API to get the properties?

Rob, any thought on this?

> +	{}
> +};
> +
> +static struct serdev_device_driver cros_ec_uart_driver = {
> +	.driver	= {
> +		.name	= "cros-ec-uart",
> +		.of_match_table = cros_ec_uart_of_match,
> +		.pm	= &cros_ec_uart_pm_ops,
> +	},
> +	.probe		= cros_ec_uart_probe,
> +	.remove		= cros_ec_uart_remove,
> +};
> +
> +module_serdev_device_driver(cros_ec_uart_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("UART interface for ChromeOS Embedded Controller");
> +MODULE_AUTHOR("Bhanu Prakash Maiya <bhanumaiya@chromium.org>");
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 2/2] dt-bindings: mfd: Add DT compatible string "google,cros_ec_uart"
  2020-07-15  7:49 Bhanu Prakash Maiya
@ 2020-07-15  7:49   ` Bhanu Prakash Maiya
  0 siblings, 0 replies; 10+ messages in thread
From: Bhanu Prakash Maiya @ 2020-07-15  7:49 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Lee Jones, Furquan Shaikh, Raul E Rangel, Eric Peers,
	Duncan Laurie, Benson Leung, Enric Balletbo i Serra,
	Bhanu Prakash Maiya, Guenter Roeck, Mauro Carvalho Chehab,
	David S . Miller, Rob Herring, Greg Kroah-Hartman, devicetree,
	linux-kernel, Bhanu Prakash Maiya

From: Bhanu Prakash Maiya <bhanumaiya@chromium.org>

Add DT compatible string in
Documentation/devicetree/bindings/mfd/cros_ec.txt

Series-to: LKML <linux-kernel@vger.kernel.org>
Series-cc: Raul E Rangel <rrangel@chromium.org>, Furquan Shaikh <furquan@chromium.org>, Duncan Laurie <dlaurie@google.com>, Eric Peers <epeers@google.com>, Benson Leung <bleung@chromium.org>, Enric Balletbo i Serra <enric.balletbo@collabora.com>, Guenter Roeck <groeck@chromium.org>, linux-kernel@vger.kernel.org, Lee Jones <lee.jones@linaro.org>, Rob Herring <robh+dt@kernel.org>, devicetree@vger.kernel.org

Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@chromium.org>
Change-Id: Icfeab15fa04daaffc61280faf5a75cd9b23ee822
Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@google.com>
---
 Documentation/devicetree/bindings/mfd/cros-ec.txt | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/cros-ec.txt b/Documentation/devicetree/bindings/mfd/cros-ec.txt
index 4860eabd0f729..ec8c5d7ecc266 100644
--- a/Documentation/devicetree/bindings/mfd/cros-ec.txt
+++ b/Documentation/devicetree/bindings/mfd/cros-ec.txt
@@ -3,7 +3,7 @@ ChromeOS Embedded Controller
 Google's ChromeOS EC is a Cortex-M device which talks to the AP and
 implements various function such as keyboard and battery charging.
 
-The EC can be connect through various means (I2C, SPI, LPC, RPMSG) and the
+The EC can be connect through various means (I2C, SPI, UART, LPC, RPMSG) and the
 compatible string used depends on the interface. Each connection method has
 its own driver which connects to the top level interface-agnostic EC driver.
 Other Linux driver (such as cros-ec-keyb for the matrix keyboard) connect to
@@ -17,6 +17,10 @@ Required properties (SPI):
 - compatible: "google,cros-ec-spi"
 - reg: SPI chip select
 
+Required properties (UART):
+- compatible: "google,cros-ec-uart"
+- reg: UART baudrate, flowcontrol
+
 Required properties (RPMSG):
 - compatible: "google,cros-ec-rpmsg"
 
@@ -72,5 +76,6 @@ spi@131b0000 {
 	};
 };
 
-
 Example for LPC is not supplied as it is not yet implemented.
+
+Example for UART is not supplied as it is not yet implemented.
-- 
2.27.0.389.gc38d7665816-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/2] dt-bindings: mfd: Add DT compatible string "google, cros_ec_uart"
@ 2020-07-15  7:49   ` Bhanu Prakash Maiya
  0 siblings, 0 replies; 10+ messages in thread
From: Bhanu Prakash Maiya @ 2020-07-15  7:49 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Rob Herring, Mauro Carvalho Chehab, Duncan Laurie, devicetree,
	Furquan Shaikh, Lee Jones, linux-kernel, Bhanu Prakash Maiya,
	David S . Miller, Raul E Rangel, Greg Kroah-Hartman,
	Enric Balletbo i Serra, Guenter Roeck, Benson Leung, Eric Peers,
	Bhanu Prakash Maiya

From: Bhanu Prakash Maiya <bhanumaiya@chromium.org>

Add DT compatible string in
Documentation/devicetree/bindings/mfd/cros_ec.txt

Series-to: LKML <linux-kernel@vger.kernel.org>
Series-cc: Raul E Rangel <rrangel@chromium.org>, Furquan Shaikh <furquan@chromium.org>, Duncan Laurie <dlaurie@google.com>, Eric Peers <epeers@google.com>, Benson Leung <bleung@chromium.org>, Enric Balletbo i Serra <enric.balletbo@collabora.com>, Guenter Roeck <groeck@chromium.org>, linux-kernel@vger.kernel.org, Lee Jones <lee.jones@linaro.org>, Rob Herring <robh+dt@kernel.org>, devicetree@vger.kernel.org

Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@chromium.org>
Change-Id: Icfeab15fa04daaffc61280faf5a75cd9b23ee822
Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@google.com>
---
 Documentation/devicetree/bindings/mfd/cros-ec.txt | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/cros-ec.txt b/Documentation/devicetree/bindings/mfd/cros-ec.txt
index 4860eabd0f729..ec8c5d7ecc266 100644
--- a/Documentation/devicetree/bindings/mfd/cros-ec.txt
+++ b/Documentation/devicetree/bindings/mfd/cros-ec.txt
@@ -3,7 +3,7 @@ ChromeOS Embedded Controller
 Google's ChromeOS EC is a Cortex-M device which talks to the AP and
 implements various function such as keyboard and battery charging.
 
-The EC can be connect through various means (I2C, SPI, LPC, RPMSG) and the
+The EC can be connect through various means (I2C, SPI, UART, LPC, RPMSG) and the
 compatible string used depends on the interface. Each connection method has
 its own driver which connects to the top level interface-agnostic EC driver.
 Other Linux driver (such as cros-ec-keyb for the matrix keyboard) connect to
@@ -17,6 +17,10 @@ Required properties (SPI):
 - compatible: "google,cros-ec-spi"
 - reg: SPI chip select
 
+Required properties (UART):
+- compatible: "google,cros-ec-uart"
+- reg: UART baudrate, flowcontrol
+
 Required properties (RPMSG):
 - compatible: "google,cros-ec-rpmsg"
 
@@ -72,5 +76,6 @@ spi@131b0000 {
 	};
 };
 
-
 Example for LPC is not supplied as it is not yet implemented.
+
+Example for UART is not supplied as it is not yet implemented.
-- 
2.27.0.389.gc38d7665816-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-07-15 15:50 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-15  8:25 [PATCH v2 1/2] cros: platform/chrome: Add cros-ec-uart driver for uart support Bhanu Prakash Maiya
2020-07-15  8:25 ` Bhanu Prakash Maiya
2020-07-15  8:25 ` [PATCH v2 2/2] dt-bindings: mfd: Add DT compatible string "google,cros_ec_uart" Bhanu Prakash Maiya
2020-07-15  8:25   ` [PATCH v2 2/2] dt-bindings: mfd: Add DT compatible string "google, cros_ec_uart" Bhanu Prakash Maiya
2020-07-15 15:40   ` [PATCH v2 2/2] dt-bindings: mfd: Add DT compatible string "google,cros_ec_uart" Enric Balletbo i Serra
2020-07-15 15:40     ` Enric Balletbo i Serra
2020-07-15 15:48 ` [PATCH v2 1/2] cros: platform/chrome: Add cros-ec-uart driver for uart support Enric Balletbo i Serra
2020-07-15 15:48   ` Enric Balletbo i Serra
  -- strict thread matches above, loose matches on Subject: below --
2020-07-15  7:49 Bhanu Prakash Maiya
2020-07-15  7:49 ` [PATCH v2 2/2] dt-bindings: mfd: Add DT compatible string "google,cros_ec_uart" Bhanu Prakash Maiya
2020-07-15  7:49   ` [PATCH v2 2/2] dt-bindings: mfd: Add DT compatible string "google, cros_ec_uart" Bhanu Prakash Maiya

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.