Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v1 13/19] hid-ft260: improve usb interface type detection logic
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
  To: chrysh, daniel.beer, jikos
  Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
	gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>

This commit simplifies the ft260_get_interface_type routine by
replacing the ft260_is_serial with iface_type and making use of
its return value as it's in the mainline ft260 driver code.

Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
 drivers/hid/hid-ft260.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 3d1a9ec88cb9..9ecd91d173d2 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -309,7 +309,7 @@ struct ft260_device {
 	struct i2c_adapter adap;
 	struct hid_device *hdev;
 
-	bool ft260_is_serial;
+	int iface_type;
 	struct list_head device_list;
 
 	/* tty_port lifetime is equal to device lifetime */
@@ -889,28 +889,25 @@ static int ft260_get_interface_type(struct hid_device *hdev, struct ft260_device
 	ft260_dbg("i2c_enable: 0x%02x\n", cfg.i2c_enable);
 	ft260_dbg("uart_mode:  0x%02x\n", cfg.uart_mode);
 
-	dev->ft260_is_serial = false;
 	dev->power_saving_en = cfg.power_saving_en;
 
 	switch (cfg.chip_mode) {
 	case FT260_MODE_ALL:
 	case FT260_MODE_BOTH:
-		if (interface == 1) {
+		if (interface == 1)
 			ret = FT260_IFACE_UART;
-			dev->ft260_is_serial = true;
-		} else {
+		else
 			ret = FT260_IFACE_I2C;
-		}
 		break;
 	case FT260_MODE_UART:
 		ret = FT260_IFACE_UART;
-		dev->ft260_is_serial = true;
 		break;
 	case FT260_MODE_I2C:
 		ret = FT260_IFACE_I2C;
 		break;
 	}
 
+	dev->iface_type = ret;
 	return ret;
 }
 
@@ -1713,15 +1710,12 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	mutex_init(&dev->lock);
 	init_completion(&dev->wait);
 
-	if (!dev->ft260_is_serial) {
+	if (ret == FT260_IFACE_I2C)
 		ret = ft260_i2c_probe(hdev, dev);
-		if (ret)
-			goto err_hid_close;
-	} else {
+	else
 		ret = ft260_uart_probe(hdev, dev);
-		if (ret)
-			goto err_hid_close;
-	}
+	if (ret)
+		goto err_hid_close;
 
 	return 0;
 
@@ -1742,7 +1736,7 @@ static void ft260_remove(struct hid_device *hdev)
 	if (!dev)
 		return;
 
-	if (dev->ft260_is_serial) {
+	if (dev->iface_type == FT260_IFACE_UART) {
 		// FIXME:
 		cancel_work_sync(&dev->wakeup_work);
 		tty_port_unregister_device(&dev->port, ft260_tty_driver,
-- 
2.40.1


^ permalink raw reply related

* [PATCH v1 14/19] hid-ft260: uart: cleanup and refactoring
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
  To: chrysh, daniel.beer, jikos
  Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
	gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>

- Cleanup printouts and comments.
- Refactor to adjust to the module's style.
- Replace the kfifo_in_locked/kfifo_out_locked with
  kfifo_in_spinlocked/kfifo_out_spinlocked since the former may be
  dropped in the future.

Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
 drivers/hid/hid-ft260.c | 148 ++++++++++++++++++----------------------
 1 file changed, 66 insertions(+), 82 deletions(-)

diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 9ecd91d173d2..1c113f735524 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * hid-ft260.c - FTDI FT260 USB HID to I2C host bridge
+ * FTDI FT260 USB HID to I2C/UART host bridge
  *
  * Copyright (c) 2021, Michael Zaidman <michaelz@xsightlabs.com>
  *
@@ -18,11 +18,7 @@
 #include <linux/kfifo.h>
 #include <linux/tty_flip.h>
 #include <linux/minmax.h>
-#include <asm/unaligned.h> /* Needed for cpu_to_le16, le16_to_cpu */
-
-#define UART_COUNT_MAX		4	/* Number of UARTs this driver can handle */
-#define FIFO_SIZE	256
-#define TTY_WAKEUP_WATERMARK	(FIFO_SIZE / 2)
+#include <asm/unaligned.h>
 
 #ifdef DEBUG
 static int ft260_debug = 1;
@@ -148,7 +144,7 @@ enum {
 	FT260_FLAG_START_STOP_REPEATED	= 0x07,
 };
 
-/* Return values for ft260_get_interface_type func */
+/* USB interface type values */
 enum {
 	FT260_IFACE_NONE,
 	FT260_IFACE_I2C,
@@ -250,18 +246,19 @@ struct ft260_input_report {
 } __packed;
 
 /* UART reports */
+
 struct ft260_uart_write_request_report {
 	u8 report;		/* FT260_UART_REPORT */
 	u8 length;		/* data payload length */
 	u8 data[FT260_WR_UART_DATA_MAX]; /* data payload */
 } __packed;
 
-struct ft260_configure_uart_request {
+struct ft260_configure_uart_request_report {
 	u8 report;		/* FT260_SYSTEM_SETTINGS */
 	u8 request;		/* FT260_SET_UART_CONFIG */
 	u8 flow_ctrl;		/* 0: OFF, 1: RTS_CTS, 2: DTR_DSR */
 				/* 3: XON_XOFF, 4: No flow ctrl */
-	/* The baudrate field is unaligned: */
+	/* The baudrate field is unaligned */
 	__le32 baudrate;	/* little endian, 9600 = 0x2580, 19200 = 0x4B00 */
 	u8 data_bit;		/* 7 or 8 */
 	u8 parity;		/* 0: no parity, 1: odd, 2: even, 3: high, 4: low */
@@ -296,7 +293,11 @@ enum {
 	FT260_CFG_BAUD_MAX		= 12000000,
 };
 
-#define FT260_UART_EN_PW_SAVE_BAUD	4800
+#define FT260_UART_EN_PW_SAVE_BAUD	(4800)
+
+#define UART_COUNT_MAX (4) /* Number of supported UARTs */
+#define XMIT_FIFO_SIZE (256)
+#define TTY_WAKEUP_WATERMARK (XMIT_FIFO_SIZE / 2)
 
 static const struct hid_device_id ft260_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_FUTURE_TECHNOLOGY,
@@ -308,23 +309,18 @@ MODULE_DEVICE_TABLE(hid, ft260_devices);
 struct ft260_device {
 	struct i2c_adapter adap;
 	struct hid_device *hdev;
-
 	int iface_type;
 	struct list_head device_list;
-
-	/* tty_port lifetime is equal to device lifetime */
 	struct tty_port port;
+	/* tty port index */
 	unsigned int index;
 	struct kfifo xmit_fifo;
-	/* write_lock: lock to serialize access to xmit fifo */
-	spinlock_t write_lock;
+	spinlock_t xmit_fifo_lock;
 	struct uart_icount icount;
-
 	struct timer_list wakeup_timer;
 	struct work_struct wakeup_work;
 	bool reschedule_work;
 	bool power_saving_en;
-
 	struct completion wait;
 	struct mutex lock;
 	u8 i2c_wr_buf[FT260_REPORT_MAX_LEN];
@@ -337,8 +333,7 @@ struct ft260_device {
 };
 
 static int ft260_hid_feature_report_get(struct hid_device *hdev,
-					unsigned char report_id, u8 *data,
-					size_t len)
+					u8 report_id, u8 *data, size_t len)
 {
 	u8 *buf;
 	int ret;
@@ -467,8 +462,6 @@ static int ft260_hid_output_report_check_status(struct ft260_device *dev,
 
 	ret = ft260_hid_output_report(hdev, data, len);
 	if (ret < 0) {
-		hid_dbg(hdev, "%s: failed to start transfer, ret %d\n",
-			__func__, ret);
 		ft260_i2c_reset(hdev);
 		return ret;
 	}
@@ -579,6 +572,8 @@ static int ft260_smbus_write(struct ft260_device *dev, u8 addr, u8 cmd,
 		  rep->report, addr, cmd, rep->length, len);
 
 	ret = ft260_hid_output_report_check_status(dev, (u8 *)rep, len);
+	if (ret < 0)
+		hid_err(dev->hdev, "%s: failed with %d\n", __func__, ret);
 
 	return ret;
 }
@@ -682,8 +677,7 @@ static int ft260_i2c_write_read(struct ft260_device *dev, struct i2c_msg *msgs)
 		else
 			read_off = *msgs[0].buf;
 
-		ft260_dbg("%s: off %#x rlen %d wlen %d\n", __func__,
-			read_off, rd_len, wr_len);
+		ft260_dbg("off %#x rlen %d wlen %d\n", read_off, rd_len, wr_len);
 	}
 
 	ret = ft260_i2c_write(dev, addr, msgs[0].buf, wr_len,
@@ -1051,9 +1045,6 @@ static const struct attribute_group ft260_attr_group = {
 	}
 };
 
-/***
- * START Serial dev part
- */
 static DEFINE_MUTEX(ft260_uart_list_lock);
 static LIST_HEAD(ft260_uart_device_list);
 
@@ -1065,7 +1056,7 @@ static void ft260_uart_wakeup_workaraund_enable(struct ft260_device *port,
 	if (port->power_saving_en) {
 		port->reschedule_work = enable;
 		ft260_dbg("%s wakeup workaround",
-			  enable ? "Activate" : "Deactivate");
+			  enable ? "activate" : "deactivate");
 	}
 }
 
@@ -1085,8 +1076,8 @@ static int ft260_uart_add_port(struct ft260_device *port)
 	int index = 0, ret = 0;
 	struct ft260_device *dev;
 
-	spin_lock_init(&port->write_lock);
-	if (kfifo_alloc(&port->xmit_fifo, FIFO_SIZE, GFP_KERNEL))
+	spin_lock_init(&port->xmit_fifo_lock);
+	if (kfifo_alloc(&port->xmit_fifo, XMIT_FIFO_SIZE, GFP_KERNEL))
 		return -ENOMEM;
 
 	mutex_lock(&ft260_uart_list_lock);
@@ -1116,9 +1107,9 @@ static void ft260_uart_port_remove(struct ft260_device *port)
 	list_del(&port->device_list);
 	mutex_unlock(&ft260_uart_list_lock);
 
-	spin_lock(&port->write_lock);
+	spin_lock(&port->xmit_fifo_lock);
 	kfifo_free(&port->xmit_fifo);
-	spin_unlock(&port->write_lock);
+	spin_unlock(&port->xmit_fifo_lock);
 
 	mutex_lock(&port->port.mutex);
 	tty_port_tty_hangup(&port->port, false);
@@ -1127,7 +1118,7 @@ static void ft260_uart_port_remove(struct ft260_device *port)
 	ft260_uart_port_put(port);
 }
 
-static struct ft260_device *ft260_uart_port_get(unsigned int index)
+static struct ft260_device *ft260_uart_port_get(int index)
 {
 	struct ft260_device *port;
 
@@ -1191,20 +1182,18 @@ static int ft260_uart_transmit_chars(struct ft260_device *port)
 		rep->report = FT260_UART_DATA_REPORT_ID(len);
 		rep->length = len;
 
-		len = kfifo_out_locked(xmit, rep->data, len, &port->write_lock);
+		len = kfifo_out_spinlocked(xmit, rep->data, len, &port->xmit_fifo_lock);
 
 		ret = ft260_hid_output_report(hdev, (u8 *)rep, len + sizeof(*rep));
-		if (ret < 0) {
-			hid_err(hdev, "Failed to start transfer, ret %d\n", ret);
+		if (ret < 0)
 			goto tty_out;
-		}
 
 		data_len -= len;
 		port->icount.tx += len;
 	} while (data_len > 0);
 
 	len = kfifo_len(xmit);
-	if ((FIFO_SIZE - len) > TTY_WAKEUP_WATERMARK)
+	if ((XMIT_FIFO_SIZE - len) > TTY_WAKEUP_WATERMARK)
 		tty_wakeup(tty);
 
 	ret = 0;
@@ -1214,15 +1203,14 @@ static int ft260_uart_transmit_chars(struct ft260_device *port)
 	return ret;
 }
 
-static int ft260_uart_receive_chars(struct ft260_device *port,
-				    u8 *data, u8 length)
+static int ft260_uart_receive_chars(struct ft260_device *port, u8 *data, u8 length)
 {
-	struct hid_device *hdev = port->hdev;
 	int ret;
 
 	ret = tty_insert_flip_string(&port->port, data, length);
 	if (ret != length)
-		hid_dbg(hdev, "%d char not inserted to flip buffer\n", length - ret);
+		ft260_dbg("%d char not inserted to flip buf\n", length - ret);
+
 	port->icount.rx += ret;
 
 	if (ret)
@@ -1231,27 +1219,25 @@ static int ft260_uart_receive_chars(struct ft260_device *port,
 	return ret;
 }
 
-static int ft260_uart_write(struct tty_struct *tty, const unsigned char *buf,
-			    int count)
+static int ft260_uart_write(struct tty_struct *tty, const u8 *buf, int cnt)
 {
 	struct ft260_device *port = tty->driver_data;
-	struct hid_device *hdev = port->hdev;
-	int len, ret;
+	int len, ret, diff;
 
-	len = kfifo_in_locked(&port->xmit_fifo, buf, count, &port->write_lock);
-	ft260_dbg("count: %d, len: %d", count, len);
+	len = kfifo_in_spinlocked(&port->xmit_fifo, buf, cnt, &port->xmit_fifo_lock);
+	ft260_dbg("count: %d, len: %d", cnt, len);
 
 	ret = ft260_uart_transmit_chars(port);
 	if (ret < 0) {
-		hid_dbg(hdev, "Failed to transmit chars: %d\n", ret);
+		ft260_dbg("failed to transmit %d\n", ret);
 		return 0;
 	}
 
 	ret = kfifo_len(&port->xmit_fifo);
 	if (ret > 0) {
-		hid_dbg(hdev, "Failed to  all kfifo data bytes\n");
-		ft260_dbg("return: %d", len - ret);
-		return len - ret;
+		diff = len - ret;
+		ft260_dbg("failed to send %d out of %d bytes\n", diff, len);
+		return diff;
 	}
 
 	return len;
@@ -1277,7 +1263,7 @@ static int ft260_uart_change_speed(struct ft260_device *port,
 {
 	struct hid_device *hdev = port->hdev;
 	unsigned int baud;
-	struct ft260_configure_uart_request req;
+	struct ft260_configure_uart_request_report req;
 	bool wakeup_workaraund = false;
 	int ret;
 
@@ -1292,7 +1278,7 @@ static int ft260_uart_change_speed(struct ft260_device *port,
 		break;
 	case CS5:
 	case CS6:
-		hid_err(hdev, "Invalid data bit size, setting to default (8 bit)\n");
+		hid_err(hdev, "invalid data bit size, setting a default\n");
 		req.data_bit = FT260_CFG_DATA_BITS_8;
 		termios->c_cflag &= ~CSIZE;
 		termios->c_cflag |= CS8;
@@ -1317,7 +1303,7 @@ static int ft260_uart_change_speed(struct ft260_device *port,
 	if (baud == 0 || baud < FT260_CFG_BAUD_MIN || baud > FT260_CFG_BAUD_MAX) {
 		struct tty_struct *tty = tty_port_tty_get(&port->port);
 
-		hid_err(hdev, "Invalid baud rate %d\n", baud);
+		hid_err(hdev, "invalid baud rate %d\n", baud);
 		baud = 9600;
 		tty_encode_baud_rate(tty, baud, baud);
 		tty_kref_put(tty);
@@ -1335,7 +1321,7 @@ static int ft260_uart_change_speed(struct ft260_device *port,
 	else
 		req.flow_ctrl = FT260_CFG_FLOW_CTRL_OFF;
 
-	ft260_dbg("Configured termios: flow control: %d, baudrate: %d, ",
+	ft260_dbg("configured termios: flow control: %d, baudrate: %d, ",
 		  req.flow_ctrl, baud);
 	ft260_dbg("data_bit: %d, parity: %d, stop_bit: %d, breaking: %d\n",
 		  req.data_bit, req.parity,
@@ -1346,7 +1332,7 @@ static int ft260_uart_change_speed(struct ft260_device *port,
 
 	ret = ft260_hid_feature_report_set(hdev, (u8 *)&req, sizeof(req));
 	if (ret < 0)
-		hid_err(hdev, "ft260_hid_feature_report_set failed: %d\n", ret);
+		hid_err(hdev, "failed to change termios: %d\n", ret);
 
 	return ret;
 }
@@ -1396,8 +1382,8 @@ static int ft260_uart_proc_show(struct seq_file *m, void *v)
 {
 	int i;
 
-	seq_printf(m, "ft260 info:1.0 driver%s%s revision:%s\n",
-			"", "", "");
+	seq_printf(m, "ft260 info:1.0 driver%s%s revision:%s\n", "", "", "");
+
 	for (i = 0; i < UART_COUNT_MAX; i++) {
 		struct ft260_device *port = ft260_uart_port_get(i);
 
@@ -1452,7 +1438,8 @@ static const struct tty_operations ft260_uart_ops = {
 	.get_icount		= ft260_uart_get_icount,
 };
 
-/* The FT260 has a "power saving mode" that causes the device to switch
+/*
+ * The FT260 has a "power saving mode" that causes the device to switch
  * to a 30 kHz oscillator if there's no activity for 5 seconds.
  * Unfortunately, this mode can only be disabled by reprogramming
  * internal fuses, which requires an additional programming voltage.
@@ -1476,16 +1463,14 @@ static void ft260_uart_start_wakeup(struct timer_list *t)
 
 static void ft260_uart_wakeup(struct ft260_device *dev)
 {
-	struct ft260_get_chip_version_report version;
+	struct ft260_get_chip_version_report ver;
 	int ret;
 
 	if (dev->reschedule_work) {
 		ret = ft260_hid_feature_report_get(dev->hdev, FT260_CHIP_VERSION,
-					  (u8 *)&version, sizeof(version));
+						   (u8 *)&ver, sizeof(ver));
 		if (ret < 0)
-			hid_err(dev->hdev,
-				"%s: failed to start transfer, ret %d\n",
-				__func__, ret);
+			hid_err(dev->hdev, "%s: failed with %d\n", __func__, ret);
 	}
 }
 
@@ -1497,7 +1482,7 @@ static void ft260_uart_do_wakeup(struct work_struct *work)
 	ft260_uart_wakeup(dev);
 }
 
-static void ft260_uart_shutdown(struct tty_port *tport)
+static void ft260_uart_port_shutdown(struct tty_port *tport)
 {
 	struct ft260_device *port =
 		container_of(tport, struct ft260_device, port);
@@ -1505,7 +1490,7 @@ static void ft260_uart_shutdown(struct tty_port *tport)
 	ft260_uart_wakeup_workaraund_enable(port, false);
 }
 
-static int ft260_uart_activate(struct tty_port *tport, struct tty_struct *tty)
+static int ft260_uart_port_activate(struct tty_port *tport, struct tty_struct *tty)
 {
 	struct ft260_device *port =
 		container_of(tport, struct ft260_device, port);
@@ -1516,9 +1501,9 @@ static int ft260_uart_activate(struct tty_port *tport, struct tty_struct *tty)
 	 */
 	set_bit(TTY_IO_ERROR, &tty->flags);
 
-	spin_lock(&port->write_lock);
+	spin_lock(&port->xmit_fifo_lock);
 	kfifo_reset(&port->xmit_fifo);
-	spin_unlock(&port->write_lock);
+	spin_unlock(&port->xmit_fifo_lock);
 
 	clear_bit(TTY_IO_ERROR, &tty->flags);
 
@@ -1542,8 +1527,8 @@ static void ft260_uart_port_destroy(struct tty_port *tport)
 }
 
 static const struct tty_port_operations ft260_uart_port_ops = {
-	.shutdown = ft260_uart_shutdown,
-	.activate = ft260_uart_activate,
+	.shutdown = ft260_uart_port_shutdown,
+	.activate = ft260_uart_port_activate,
 	.destruct = ft260_uart_port_destroy,
 };
 
@@ -1595,7 +1580,7 @@ static int ft260_i2c_probe(struct hid_device *hdev, struct ft260_device *dev)
 
 static int ft260_uart_probe(struct hid_device *hdev, struct ft260_device *dev)
 {
-	struct ft260_configure_uart_request req;
+	struct ft260_configure_uart_request_report req;
 	int ret;
 	struct device *devt;
 
@@ -1623,10 +1608,10 @@ static int ft260_uart_probe(struct hid_device *hdev, struct ft260_device *dev)
 		ret = PTR_ERR(devt);
 		goto err_register_tty;
 	}
-	hid_info(hdev, "Registering device /dev/%s%d\n",
+	hid_info(hdev, "registering device /dev/%s%d\n",
 		ft260_tty_driver->name, dev->index);
 
-	/* Send Feature Report to Configure FT260 as UART 9600-8-N-1 */
+	/* Configure UART to 9600n8 */
 	req.report	= FT260_SYSTEM_SETTINGS;
 	req.request	= FT260_SET_UART_CONFIG;
 	req.flow_ctrl	= FT260_CFG_FLOW_CTRL_NONE;
@@ -1638,8 +1623,7 @@ static int ft260_uart_probe(struct hid_device *hdev, struct ft260_device *dev)
 
 	ret = ft260_hid_feature_report_set(hdev, (u8 *)&req, sizeof(req));
 	if (ret < 0) {
-		hid_err(hdev, "ft260_hid_feature_report_set failed: %d\n",
-			ret);
+		hid_err(hdev, "failed to configure uart: %d\n", ret);
 		goto err_hid_report;
 	}
 
@@ -1660,9 +1644,9 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	if (!hid_is_usb(hdev))
 		return -EINVAL;
-
-	/* We cannot used devm_kzalloc here, because port has to survive until
-	 * destroy function call
+	/*
+	 * We cannot use devm_kzalloc here because the port has to survive
+	 * until destroy function call.
 	 */
 	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
 	if (!dev) {
@@ -1742,7 +1726,7 @@ static void ft260_remove(struct hid_device *hdev)
 		tty_port_unregister_device(&dev->port, ft260_tty_driver,
 					   dev->index);
 		ft260_uart_port_remove(dev);
-		/* dev still needed, so we will free it in _destroy func */
+		/* dev is still needed, so we will free it in _destroy func */
 	} else {
 		sysfs_remove_group(&hdev->dev.kobj, &ft260_attr_group);
 		i2c_del_adapter(&dev->adap);
@@ -1781,7 +1765,7 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
 		return 0;
 
 	} else if (xfer->length > FT260_RD_DATA_MAX) {
-		hid_err(hdev, "Received data too long (%d)\n", xfer->length);
+		hid_err(hdev, "received data too long (%d)\n", xfer->length);
 		return -EBADR;
 	} else if (xfer->report >= FT260_UART_REPORT_MIN &&
 		   xfer->report <= FT260_UART_REPORT_MAX) {
@@ -1830,7 +1814,7 @@ static int __init ft260_driver_init(void)
 		goto err_reg_driver;
 	}
 
-	ret = hid_register_driver(&(ft260_driver));
+	ret = hid_register_driver(&ft260_driver);
 	if (ret) {
 		pr_err("hid_register_driver failed: %d\n", ret);
 		goto err_reg_hid;
@@ -1848,7 +1832,7 @@ static int __init ft260_driver_init(void)
 
 static void __exit ft260_driver_exit(void)
 {
-	hid_unregister_driver(&(ft260_driver));
+	hid_unregister_driver(&ft260_driver);
 	tty_unregister_driver(ft260_tty_driver);
 	tty_driver_kref_put(ft260_tty_driver);
 }
-- 
2.40.1


^ permalink raw reply related

* [PATCH v1 15/19] hid-ft260: uart: remove FIXME for wake-up workaround
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
  To: chrysh, daniel.beer, jikos
  Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
	gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>

The FIXME is related to the wake-up workaround cancelation - do we
need the reschedule_work flag alongside the cancel_work_sync usage?

The teardown sequence is as described below:

Upon tty session termination:
   ft260_uart_port_shutdown
       ft260_uart_wakeup_workaraund_enable: deactivate wakeup workaround
   t260_uart_cleanup
   ft260_uart_port_put

On rmmod:
   ft260_remove
       cancel_work_sync
           ft260_uart_port_remove
               timer_delete_sync
   ft260_uart_port_put
   ft260_uart_port_destroy

The ft260_uart_start_wakeup timer_work can occur after the cancel_work_sync
returns, rescheduling the wakeup_work again. The reschedule_work flag set
earlier at the ft260_uart_port_shutdown time prevents it.

The alternative could be performing the timer_delete_sync first, preventing
the wakeup_timer and wakeup_work from rescheduling, and then canceling the
last wakeup_work by calling the cancel_work_sync. However, we still need the
reschedule_work flag to enable or bypass the power saving mode according to
the requested baud rate.

Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
 drivers/hid/hid-ft260.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 1c113f735524..52ccee83250e 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1585,7 +1585,6 @@ static int ft260_uart_probe(struct hid_device *hdev, struct ft260_device *dev)
 	struct device *devt;
 
 	INIT_WORK(&dev->wakeup_work, ft260_uart_do_wakeup);
-	// FIXME: Do I need that if I have cancel_work_sync?
 	// FIXME: are all kfifo access secured by lock? with irq or not?
 	ft260_uart_wakeup_workaraund_enable(dev, true);
 	/* Work not started at this point */
@@ -1721,7 +1720,6 @@ static void ft260_remove(struct hid_device *hdev)
 		return;
 
 	if (dev->iface_type == FT260_IFACE_UART) {
-		// FIXME:
 		cancel_work_sync(&dev->wakeup_work);
 		tty_port_unregister_device(&dev->port, ft260_tty_driver,
 					   dev->index);
-- 
2.40.1


^ permalink raw reply related

* [PATCH v1 16/19] hid-ft260: uart: suppress unhandled report 0xb1 dmesg
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
  To: chrysh, daniel.beer, jikos
  Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
	gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>

Suppress the "unhandled report 0xb1" error since it's related to the UART
DCD/RI function of the multifunctional GPIO pins status, which we do not
use for serial console. The configuration of these pins is a part
of the GPIO patch set.

[ 5453.117113] ft260 0003:0403:6030.0008: unhandled report 0xb1
[ 6641.582307] ft260 0003:0403:6030.0008: unhandled report 0xb1
[13418.439085] ft260 0003:0403:6030.0008: unhandled report 0xb1
[14110.820786] ft260 0003:0403:6030.0008: unhandled report 0xb1

Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
 drivers/hid/hid-ft260.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 52ccee83250e..d7eb00aeb669 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1768,6 +1768,8 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
 	} else if (xfer->report >= FT260_UART_REPORT_MIN &&
 		   xfer->report <= FT260_UART_REPORT_MAX) {
 		return ft260_uart_receive_chars(dev, xfer->data, xfer->length);
+	} else if (xfer->report == FT260_UART_INTERRUPT_STATUS) {
+		return 0;
 	}
 	hid_err(hdev, "unhandled report %#02x\n", xfer->report);
 
-- 
2.40.1


^ permalink raw reply related

* [PATCH v1 17/19] hid-ft260: uart: arm wake-up timer unconditionally on tty session start
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
  To: chrysh, daniel.beer, jikos
  Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
	gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>

The previous session deactivated the wake-up workaround. The next session calls
ft260_uart_port_activate while the wake-up workaround flag is yet deactivated.
It is enabled later, conditionally, in the ft260_uart_change_speed. Thus, the
timer is never armed again, and the device enters power-saving mode and misses
the incoming data.

Arming the wake-up timer unconditionally on the tty session start resolved the
issue.

Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
 drivers/hid/hid-ft260.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index d7eb00aeb669..7f3ef4f20075 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1510,10 +1510,8 @@ static int ft260_uart_port_activate(struct tty_port *tport, struct tty_struct *t
 	/* Wake up the chip as early as possible to not miss incoming data */
 	ft260_uart_wakeup(port);
 
-	if (port->reschedule_work) {
-		mod_timer(&port->wakeup_timer, jiffies +
-			  msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS));
-	}
+	mod_timer(&port->wakeup_timer, jiffies +
+		  msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS));
 
 	return 0;
 }
-- 
2.40.1


^ permalink raw reply related

* [PATCH v1 18/19] hid-ft260: uart: fix rx data loss after device reopening
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
  To: chrysh, daniel.beer, jikos
  Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
	gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>

The port setting may remain intact after session termination. Then, when
reopening the port without configuring the port setting, the wakeup
mechanism is not activated, and UART Rx loses the data at speeds greater
than 4800 bauds. To fix it, retrieve the baud rate from the device in the
ft260_uart_port_activate to reactivate the wakeup workaround if needed.

Example:

1. Configure the baud rate to 115200:

   $ sudo picocom -f n -p n -d 8 -b 115200 /dev/ttyFT0

2. Quit the picocom without resetting port setting by entering:
   C-a and C-q

It deactivates the wakeup workaround:

[31677.005464] ft260_uart_close:
[31677.005466] ft260_uart_chars_in_buffer:
[31677.005467] ft260_uart_port_shutdown:
[31677.005468] ft260_uart_wakeup_workaraund_enable: deactivate wakeup workaround
[31677.005476] ft260_uart_cleanup:
[31677.005477] ft260_uart_port_put:

3. Reopen the ttyFT0 port, but do not configure it. The fix retrieves the port
   baud rate from the device, compares it against the threshold, and activates
   the wakeup mechanism when needed.

$ sudo bash -c "cat < /dev/ttyFT0"

[31693.304991] ft260_uart_port_get:
[31693.304995] ft260_uart_install:
[31693.305000] ft260_uart_open:
[31693.305001] ft260_uart_port_activate:
[31693.309842] ft260_uart_wakeup_workaraund_enable: activate wakeup workaround
[31693.309847] ft260_uart_port_activate: configurd baudrate = 115200
[31698.132650] ft260_uart_start_wakeup:
[31698.132809] ft260_uart_do_wakeup:
[31698.132817] ft260_uart_wakeup:
[31702.996905] ft260_uart_start_wakeup:
[31702.996916] ft260_uart_do_wakeup:

Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
 drivers/hid/hid-ft260.c | 56 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 47 insertions(+), 9 deletions(-)

diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 7f3ef4f20075..6b172bfa4f98 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -90,7 +90,7 @@ enum {
 	FT260_I2C_REPORT_MAX		= 0xDE,
 	FT260_GPIO			= 0xB0,
 	FT260_UART_INTERRUPT_STATUS	= 0xB1,
-	FT260_UART_STATUS		= 0xE0,
+	FT260_UART_SETTINGS		= 0xE0,
 	FT260_UART_RI_DCD_STATUS	= 0xE1,
 	FT260_UART_REPORT_MIN		= 0xF0,
 	FT260_UART_REPORT_MAX		= 0xFE,
@@ -190,6 +190,18 @@ struct ft260_get_i2c_status_report {
 	u8 reserved;
 } __packed;
 
+struct ft260_get_uart_settings_report {
+	u8 report;		/* FT260_UART_SETTINGS */
+	u8 flow_ctrl;		/* 0 - OFF; 1 - RTS_CTS, 2 - DTR_DSR, */
+				/* 3 - XON_XOFF, 4 - No flow control */
+	/* The baudrate field is unaligned */
+	__le32 baudrate;	/* little endian, 9600 = 0x2580, 19200 = 0x4B00 */
+	u8 data_bit;		/* 7 or 8 */
+	u8 parity;		/* 0: no parity, 1: odd, 2: even, 3: high, 4: low */
+	u8 stop_bit;		/* 0: one stop bit, 2: 2 stop bits */
+	u8 breaking;		/* 0: no break */
+} __packed;
+
 /* Feature Out reports */
 
 struct ft260_set_system_clock_report {
@@ -1050,6 +1062,21 @@ static LIST_HEAD(ft260_uart_device_list);
 
 static void ft260_uart_wakeup(struct ft260_device *dev);
 
+static int ft260_get_uart_settings(struct hid_device *hdev,
+				   struct ft260_get_uart_settings_report *cfg)
+{
+	int ret;
+	int len = sizeof(struct ft260_get_uart_settings_report);
+
+	ret = ft260_hid_feature_report_get(hdev, FT260_UART_SETTINGS,
+					   (u8 *)cfg, len);
+	if (ret < 0) {
+		hid_err(hdev, "failed to retrieve uart settings\n");
+		return ret;
+	}
+	return 0;
+}
+
 static void ft260_uart_wakeup_workaraund_enable(struct ft260_device *port,
 						bool enable)
 {
@@ -1492,13 +1519,11 @@ static void ft260_uart_port_shutdown(struct tty_port *tport)
 
 static int ft260_uart_port_activate(struct tty_port *tport, struct tty_struct *tty)
 {
-	struct ft260_device *port =
-		container_of(tport, struct ft260_device, port);
+	int ret;
+	int baudrate;
+	struct ft260_get_uart_settings_report cfg;
+	struct ft260_device *port = container_of(tport, struct ft260_device, port);
 
-	/*
-	 * Set the TTY IO error marker - we will only clear this
-	 * once we have successfully opened the port.
-	 */
 	set_bit(TTY_IO_ERROR, &tty->flags);
 
 	spin_lock(&port->xmit_fifo_lock);
@@ -1507,8 +1532,21 @@ static int ft260_uart_port_activate(struct tty_port *tport, struct tty_struct *t
 
 	clear_bit(TTY_IO_ERROR, &tty->flags);
 
-	/* Wake up the chip as early as possible to not miss incoming data */
-	ft260_uart_wakeup(port);
+	/*
+	 * The port setting may remain intact after session termination.
+	 * Then, when reopening the port without configuring the port
+	 * setting, we need to retrieve the baud rate from the device to
+	 * reactivate the wakeup workaround if needed.
+	 */
+	ret = ft260_get_uart_settings(port->hdev, &cfg);
+	if (ret)
+		return ret;
+
+	baudrate = get_unaligned_le32(&cfg.baudrate);
+	if (baudrate > FT260_UART_EN_PW_SAVE_BAUD)
+		ft260_uart_wakeup_workaraund_enable(port, true);
+
+	ft260_dbg("configurd baudrate = %d", baudrate);
 
 	mod_timer(&port->wakeup_timer, jiffies +
 		  msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS));
-- 
2.40.1


^ permalink raw reply related

* [PATCH v1 19/19] hid-ft260: uart: improve write performance
From: Michael Zaidman @ 2024-02-10 21:51 UTC (permalink / raw)
  To: chrysh, daniel.beer, jikos
  Cc: linux-kernel, linux-input, linux-serial, ilpo.jarvinen, johan,
	gregkh, equinox, michael.zaidman
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>

Tx performance with the current buffer size of 256 bytes is lower when
the data length exceeds the xmit buf size.

[134331.147978] ft260_uart_write: count: 288, len: 256
[134331.157945] ft260_uart_write: count: 32, len: 32
[134331.159977] ft260_uart_write: count: 288, len: 256
[134331.169990] ft260_uart_write: count: 32, len: 32

1. Increase the xmit buffer size to page size as used in the serial core
and other tty drivers.

2. Remove the xmit buffer fulness against the watermark checking and the
tty_wakeup calling in the ft260_uart_transmit_chars routine. This code is
taken from other drivers, but other drivers may call the routine from the
interrupt context. In our case, this condition is always True since xmit
buffer filling and emptying are serialized and done synchronously.

Tested with picocom ASCII file transfer by 288-byte chunks at 921600
bauds rate with above 20% performance improvement.

Before:
2821.7 Kbytes transferred at 47367 CPS... Done.

After:
2821.7 Kbytes transferred at 57788 CPS... Done.

Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
 drivers/hid/hid-ft260.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 6b172bfa4f98..1188b8e09938 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -308,8 +308,7 @@ enum {
 #define FT260_UART_EN_PW_SAVE_BAUD	(4800)
 
 #define UART_COUNT_MAX (4) /* Number of supported UARTs */
-#define XMIT_FIFO_SIZE (256)
-#define TTY_WAKEUP_WATERMARK (XMIT_FIFO_SIZE / 2)
+#define XMIT_FIFO_SIZE (PAGE_SIZE)
 
 static const struct hid_device_id ft260_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_FUTURE_TECHNOLOGY,
@@ -1211,7 +1210,7 @@ static int ft260_uart_transmit_chars(struct ft260_device *port)
 
 		len = kfifo_out_spinlocked(xmit, rep->data, len, &port->xmit_fifo_lock);
 
-		ret = ft260_hid_output_report(hdev, (u8 *)rep, len + sizeof(*rep));
+		ret = ft260_hid_output_report(hdev, (u8 *)rep, len + 2);
 		if (ret < 0)
 			goto tty_out;
 
@@ -1219,10 +1218,6 @@ static int ft260_uart_transmit_chars(struct ft260_device *port)
 		port->icount.tx += len;
 	} while (data_len > 0);
 
-	len = kfifo_len(xmit);
-	if ((XMIT_FIFO_SIZE - len) > TTY_WAKEUP_WATERMARK)
-		tty_wakeup(tty);
-
 	ret = 0;
 
 tty_out:
@@ -1546,7 +1541,7 @@ static int ft260_uart_port_activate(struct tty_port *tport, struct tty_struct *t
 	if (baudrate > FT260_UART_EN_PW_SAVE_BAUD)
 		ft260_uart_wakeup_workaraund_enable(port, true);
 
-	ft260_dbg("configurd baudrate = %d", baudrate);
+	ft260_dbg("configured baudrate = %d", baudrate);
 
 	mod_timer(&port->wakeup_timer, jiffies +
 		  msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS));
-- 
2.40.1


^ permalink raw reply related

* [RFC PATCH v2 0/6] support for Marvell 88PM886 PMIC
From: Karel Balej @ 2024-02-11  9:35 UTC (permalink / raw)
  To: Karel Balej, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lee Jones, Liam Girdwood, Mark Brown, linux-input,
	devicetree, linux-kernel
  Cc: Duje Mihanović, ~postmarketos/upstreaming, phone-devel

From: Karel Balej <balejk@matfyz.cz>

Hello,

the following implements basic support for Marvell's 88PM886 PMIC which
is found for instance as a component of the samsung,coreprimevelte
smartphone which inspired this and also serves as a testing platform.

The code for the MFD is based primarily on this old series [1] with the
addition of poweroff based on the smartphone's downstream kernel tree
[2]. The onkey and regulators drivers are based on the latter. I am not
in possesion of the datasheet.

[1] https://lore.kernel.org/all/1434098601-3498-1-git-send-email-yizhang@marvell.com/
[2] https://github.com/CoderCharmander/g361f-kernel

Thank you and kind regards,
K. B.
---
RFC v2:
- Merge with the regulators series to have multiple devices and thus
  justify the use of the MFD framework.
- Rebase on v6.8-rc3.
- Reorder patches.
- MFD RFC v1: https://lore.kernel.org/all/20231217131838.7569-1-karelb@gimli.ms.mff.cuni.cz/
- regulators RFC v1: https://lore.kernel.org/all/20231228100208.2932-1-karelb@gimli.ms.mff.cuni.cz/

Karel Balej (6):
  dt-bindings: mfd: add entry for Marvell 88PM886 PMIC
  mfd: add driver for Marvell 88PM886 PMIC
  regulator: add regulators driver for Marvell 88PM886 PMIC
  dt-bindings: input: add entry for Marvell 88PM88X PMICs onkey
  input: add onkey driver for Marvell 88PM88X PMICs
  MAINTAINERS: add myself for Marvell 88PM886 PMIC

 .../bindings/input/marvell,88pm88x-onkey.yaml |  32 +++
 .../bindings/mfd/marvell,88pm88x.yaml         |  82 +++++++
 .../regulator/marvell,88pm88x-regulator.yaml  |  28 +++
 MAINTAINERS                                   |  11 +
 drivers/input/misc/88pm88x-onkey.c            |  95 ++++++++
 drivers/input/misc/Kconfig                    |   7 +
 drivers/input/misc/Makefile                   |   1 +
 drivers/mfd/88pm88x.c                         | 211 ++++++++++++++++++
 drivers/mfd/Kconfig                           |  12 +
 drivers/mfd/Makefile                          |   1 +
 drivers/regulator/88pm88x-regulator.c         | 206 +++++++++++++++++
 drivers/regulator/Kconfig                     |   6 +
 drivers/regulator/Makefile                    |   1 +
 include/linux/mfd/88pm88x.h                   |  46 ++++
 14 files changed, 739 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
 create mode 100644 Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml
 create mode 100644 drivers/input/misc/88pm88x-onkey.c
 create mode 100644 drivers/mfd/88pm88x.c
 create mode 100644 drivers/regulator/88pm88x-regulator.c
 create mode 100644 include/linux/mfd/88pm88x.h

-- 
2.43.0


^ permalink raw reply

* [RFC PATCH v2 2/6] mfd: add driver for Marvell 88PM886 PMIC
From: Karel Balej @ 2024-02-11  9:35 UTC (permalink / raw)
  To: Karel Balej, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lee Jones, Liam Girdwood, Mark Brown, linux-input,
	devicetree, linux-kernel
  Cc: Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <20240211094609.2223-1-karelb@gimli.ms.mff.cuni.cz>

From: Karel Balej <balejk@matfyz.cz>

Marvell 88PM886 is a PMIC which provides various functions such as
onkey, battery, charger and regulators. It is found for instance in the
samsung,coreprimevelte smartphone with which this was tested.

Only implement basic support to allow for the use of regulators and
onkey omitting the currently unused register definitions and I2C
subclients which should thus be added with the subdevice drivers which
need them.

The register mapping and functions of 88PM886 are very similar to those
of 88PM880 and the downstream version of the driver handles both of
these devices. Possible future efforts to support 88PM880 should thus
make use of this driver.

Signed-off-by: Karel Balej <balejk@matfyz.cz>
---

Notes:
    RFC v2:
    - Remove some abstraction.
    - Sort includes alphabetically and add linux/of.h.
    - Depend on OF, remove of_match_ptr and add MODULE_DEVICE_TABLE.
    - Use more temporaries and break long lines.
    - Do not initialize ret in probe.
    - Use the wakeup-source DT property.
    - Rename ret to err.
    - Address Lee's comments:
      - Drop patched in presets for base regmap and related defines.
      - Use full sentences in comments.
      - Remove IRQ comment.
      - Define regmap_config member values.
      - Rename data to sys_off_data.
      - Add _PMIC suffix to Kconfig.
      - Use dev_err_probe.
      - Do not store irq_data.
      - s/WHOAMI/CHIP_ID
      - Drop LINUX part of include guard name.
      - Merge in the regulator series modifications in order to have more
        devices and modify the commit message accordingly. Changes with
        respect to the original regulator series patches:
        - ret -> err
        - Add temporary for dev in pm88x_initialize_subregmaps.
        - Drop of_compatible for the regulators.
        - Do not duplicate LDO regmap for bucks.
    - Rewrite commit message.

 drivers/mfd/88pm88x.c       | 211 ++++++++++++++++++++++++++++++++++++
 drivers/mfd/Kconfig         |  12 ++
 drivers/mfd/Makefile        |   1 +
 include/linux/mfd/88pm88x.h |  46 ++++++++
 4 files changed, 270 insertions(+)
 create mode 100644 drivers/mfd/88pm88x.c
 create mode 100644 include/linux/mfd/88pm88x.h

diff --git a/drivers/mfd/88pm88x.c b/drivers/mfd/88pm88x.c
new file mode 100644
index 000000000000..301e3e8f26f4
--- /dev/null
+++ b/drivers/mfd/88pm88x.c
@@ -0,0 +1,211 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/i2c.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/notifier.h>
+#include <linux/of.h>
+#include <linux/reboot.h>
+#include <linux/regmap.h>
+
+#include <linux/mfd/88pm88x.h>
+
+#define PM88X_REG_INT_STATUS1			0x05
+
+#define PM88X_REG_INT_ENA_1			0x0a
+#define PM88X_INT_ENA1_ONKEY			BIT(0)
+
+#define PM88X_REGMAP_CONF_REG_BITS		8
+#define PM88X_REGMAP_CONF_VAL_BITS		8
+#define PM88X_REGMAP_CONF_MAX_REG		0xfe
+
+enum pm88x_irq_number {
+	PM88X_IRQ_ONKEY,
+
+	PM88X_MAX_IRQ
+};
+
+static struct regmap_irq pm88x_regmap_irqs[] = {
+	REGMAP_IRQ_REG(PM88X_IRQ_ONKEY, 0, PM88X_INT_ENA1_ONKEY),
+};
+
+static struct regmap_irq_chip pm88x_regmap_irq_chip = {
+	.name = "88pm88x",
+	.irqs = pm88x_regmap_irqs,
+	.num_irqs = ARRAY_SIZE(pm88x_regmap_irqs),
+	.num_regs = 4,
+	.status_base = PM88X_REG_INT_STATUS1,
+	.ack_base = PM88X_REG_INT_STATUS1,
+	.unmask_base = PM88X_REG_INT_ENA_1,
+};
+
+static struct resource pm88x_onkey_resources[] = {
+	DEFINE_RES_IRQ_NAMED(PM88X_IRQ_ONKEY, "88pm88x-onkey"),
+};
+
+static struct mfd_cell pm886_devs[] = {
+	{
+		.name = "88pm88x-onkey",
+		.of_compatible = "marvell,88pm88x-onkey",
+		.num_resources = ARRAY_SIZE(pm88x_onkey_resources),
+		.resources = pm88x_onkey_resources,
+	},
+	{
+		.name = "88pm88x-regulator",
+		.id = PM88X_REGULATOR_ID_LDO2,
+	},
+	{
+		.name = "88pm88x-regulator",
+		.id = PM88X_REGULATOR_ID_LDO15,
+	},
+	{
+		.name = "88pm88x-regulator",
+		.id = PM886_REGULATOR_ID_BUCK2,
+	},
+};
+
+static const struct regmap_config pm88x_i2c_regmap = {
+	.reg_bits = PM88X_REGMAP_CONF_REG_BITS,
+	.val_bits = PM88X_REGMAP_CONF_VAL_BITS,
+	.max_register = PM88X_REGMAP_CONF_MAX_REG,
+};
+
+static int pm88x_power_off_handler(struct sys_off_data *sys_off_data)
+{
+	struct pm88x_chip *chip = sys_off_data->cb_data;
+	struct regmap *regmap = chip->regmaps[PM88X_REGMAP_BASE];
+	struct device *dev = &chip->client->dev;
+	int err;
+
+	err = regmap_update_bits(regmap, PM88X_REG_MISC_CONFIG1, PM88X_SW_PDOWN,
+				PM88X_SW_PDOWN);
+	if (err) {
+		dev_err(dev, "Failed to power off the device: %d\n", err);
+		return NOTIFY_BAD;
+	}
+	return NOTIFY_DONE;
+}
+
+static int pm88x_initialize_subregmaps(struct pm88x_chip *chip)
+{
+	struct device *dev = &chip->client->dev;
+	struct i2c_client *page;
+	struct regmap *regmap;
+	int err;
+
+	/* LDO page */
+	page = devm_i2c_new_dummy_device(dev, chip->client->adapter,
+				chip->client->addr + PM88X_PAGE_OFFSET_LDO);
+	if (IS_ERR(page)) {
+		err = PTR_ERR(page);
+		dev_err(dev, "Failed to initialize LDO client: %d\n", err);
+		return err;
+	}
+	regmap = devm_regmap_init_i2c(page, &pm88x_i2c_regmap);
+	if (IS_ERR(regmap)) {
+		err = PTR_ERR(regmap);
+		dev_err(dev, "Failed to initialize LDO regmap: %d\n", err);
+		return err;
+	}
+	chip->regmaps[PM88X_REGMAP_LDO] = regmap;
+
+	return 0;
+}
+
+static int pm88x_setup_irq(struct pm88x_chip *chip,
+		struct regmap_irq_chip_data **irq_data)
+{
+	struct regmap *regmap = chip->regmaps[PM88X_REGMAP_BASE];
+	struct device *dev = &chip->client->dev;
+	int err;
+
+	/* Set interrupt clearing mode to clear on write. */
+	err = regmap_update_bits(regmap, PM88X_REG_MISC_CONFIG2,
+			PM88X_INT_INV | PM88X_INT_CLEAR | PM88X_INT_MASK_MODE,
+			PM88X_INT_WC);
+	if (err) {
+		dev_err(dev, "Failed to set interrupt clearing mode: %d\n", err);
+		return err;
+	}
+
+	err = devm_regmap_add_irq_chip(dev, regmap, chip->client->irq,
+					IRQF_ONESHOT, -1, &pm88x_regmap_irq_chip,
+					irq_data);
+	if (err) {
+		dev_err(dev, "Failed to request IRQ: %d\n", err);
+		return err;
+	}
+
+	return 0;
+}
+
+static int pm88x_probe(struct i2c_client *client)
+{
+	struct regmap_irq_chip_data *irq_data;
+	struct device *dev = &client->dev;
+	struct pm88x_chip *chip;
+	struct regmap *regmap;
+	unsigned int chip_id;
+	int err;
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->client = client;
+	chip->whoami = (uintptr_t)device_get_match_data(dev);
+	i2c_set_clientdata(client, chip);
+
+	regmap = devm_regmap_init_i2c(client, &pm88x_i2c_regmap);
+	if (IS_ERR(regmap)) {
+		err = PTR_ERR(regmap);
+		return dev_err_probe(dev, err, "Failed to initialize regmap\n");
+	}
+	chip->regmaps[PM88X_REGMAP_BASE] = regmap;
+
+	err = regmap_read(regmap, PM88X_REG_ID, &chip_id);
+	if (err)
+		return dev_err_probe(dev, err, "Failed to read chip ID\n");
+	if (chip->whoami != chip_id)
+		return dev_err_probe(dev, -EINVAL, "Device reported wrong chip ID: %u\n",
+					chip_id);
+
+	err = pm88x_initialize_subregmaps(chip);
+	if (err)
+		return err;
+
+	err = pm88x_setup_irq(chip, &irq_data);
+	if (err)
+		return err;
+
+	err = devm_mfd_add_devices(dev, 0, pm886_devs, ARRAY_SIZE(pm886_devs),
+				NULL, 0, regmap_irq_get_domain(irq_data));
+	if (err)
+		return dev_err_probe(dev, err, "Failed to add devices\n");
+
+	err = devm_register_power_off_handler(dev, pm88x_power_off_handler, chip);
+	if (err)
+		return dev_err_probe(dev, err, "Failed to register power off handler\n");
+
+	device_init_wakeup(dev, device_property_read_bool(dev, "wakeup-source"));
+
+	return 0;
+}
+
+const struct of_device_id pm88x_of_match[] = {
+	{ .compatible = "marvell,88pm886-a1", .data = (void *)PM886_A1_CHIP_ID },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, pm88x_of_match);
+
+static struct i2c_driver pm88x_i2c_driver = {
+	.driver = {
+		.name = "88pm88x",
+		.of_match_table = pm88x_of_match,
+	},
+	.probe = pm88x_probe,
+};
+module_i2c_driver(pm88x_i2c_driver);
+
+MODULE_DESCRIPTION("Marvell 88PM88X PMIC driver");
+MODULE_AUTHOR("Karel Balej <balejk@matfyz.cz>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index e7a6e45b9fac..93ae5280fc3f 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -794,6 +794,18 @@ config MFD_88PM860X
 	  select individual components like voltage regulators, RTC and
 	  battery-charger under the corresponding menus.
 
+config MFD_88PM88X_PMIC
+	bool "Marvell 88PM886 PMIC"
+	depends on I2C=y
+	depends on OF
+	select REGMAP_I2C
+	select REGMAP_IRQ
+	select MFD_CORE
+	help
+	  This enables support for Marvell 88PM886 Power Management IC.
+	  This includes the I2C driver and the core APIs _only_, you have to
+	  select individual components like onkey under the corresponding menus.
+
 config MFD_MAX14577
 	tristate "Maxim Semiconductor MAX14577/77836 MUIC + Charger Support"
 	depends on I2C
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index c66f07edcd0e..c4f95a28f589 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -7,6 +7,7 @@
 obj-$(CONFIG_MFD_88PM860X)	+= 88pm860x.o
 obj-$(CONFIG_MFD_88PM800)	+= 88pm800.o 88pm80x.o
 obj-$(CONFIG_MFD_88PM805)	+= 88pm805.o 88pm80x.o
+obj-$(CONFIG_MFD_88PM88X_PMIC)	+= 88pm88x.o
 obj-$(CONFIG_MFD_ACT8945A)	+= act8945a.o
 obj-$(CONFIG_MFD_SM501)		+= sm501.o
 obj-$(CONFIG_ARCH_BCM2835)	+= bcm2835-pm.o
diff --git a/include/linux/mfd/88pm88x.h b/include/linux/mfd/88pm88x.h
new file mode 100644
index 000000000000..f1eaaecc784e
--- /dev/null
+++ b/include/linux/mfd/88pm88x.h
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __MFD_88PM88X_H
+#define __MFD_88PM88X_H
+
+#include <linux/mfd/core.h>
+
+#define PM886_A1_CHIP_ID		0xa1
+
+#define PM88X_REG_ID			0x00
+
+#define PM88X_REG_STATUS1		0x01
+#define PM88X_ONKEY_STS1		BIT(0)
+
+#define PM88X_REG_MISC_CONFIG1		0x14
+#define PM88X_SW_PDOWN			BIT(5)
+
+#define PM88X_REG_MISC_CONFIG2		0x15
+#define PM88X_INT_INV			BIT(0)
+#define PM88X_INT_CLEAR			BIT(1)
+#define PM88X_INT_RC			0x00
+#define PM88X_INT_WC			BIT(1)
+#define PM88X_INT_MASK_MODE		BIT(2)
+
+#define PM88X_PAGE_OFFSET_LDO		1
+
+enum pm88x_regulator_id {
+	PM88X_REGULATOR_ID_LDO2,
+	PM88X_REGULATOR_ID_LDO15,
+	PM886_REGULATOR_ID_BUCK2,
+
+	PM88X_REGULATOR_ID_SENTINEL
+};
+
+enum pm88x_regmap_index {
+	PM88X_REGMAP_BASE,
+	PM88X_REGMAP_LDO,
+
+	PM88X_REGMAP_NR
+};
+
+struct pm88x_chip {
+	struct i2c_client *client;
+	unsigned int whoami;
+	struct regmap *regmaps[PM88X_REGMAP_NR];
+};
+#endif /* __MFD_88PM88X_H */
-- 
2.43.0


^ permalink raw reply related

* [RFC PATCH v2 1/6] dt-bindings: mfd: add entry for Marvell 88PM886 PMIC
From: Karel Balej @ 2024-02-11  9:35 UTC (permalink / raw)
  To: Karel Balej, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lee Jones, Liam Girdwood, Mark Brown, linux-input,
	devicetree, linux-kernel
  Cc: Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <20240211094609.2223-1-karelb@gimli.ms.mff.cuni.cz>

From: Karel Balej <balejk@matfyz.cz>

Marvell 88PM886 is a PMIC with several subdevices such as onkey,
regulators or battery and charger. It comes in at least two revisions,
A0 and A1 -- only A1 is described here at the moment.

Signed-off-by: Karel Balej <balejk@matfyz.cz>
---

Notes:
    RFC v2:
    - Address Rob's feedback:
      - Drop mention of 88PM880.
      - Make sure the file passes bindings check (add the necessary header
        and fix `interrupt-cells`).
      - Other small changes.
    - Add regulators. Changes with respect to the regulator RFC series:
      - Address Krzysztof's comments:
        - Drop unused compatible.
        - Fix sub-node pattern.

 .../bindings/mfd/marvell,88pm88x.yaml         | 74 +++++++++++++++++++
 .../regulator/marvell,88pm88x-regulator.yaml  | 28 +++++++
 2 files changed, 102 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
 create mode 100644 Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml

diff --git a/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
new file mode 100644
index 000000000000..29ab979862d5
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
@@ -0,0 +1,74 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/marvell,88pm88x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell 88PM88X PMIC core
+
+maintainers:
+  - Karel Balej <balejk@matfyz.cz>
+
+description:
+  Marvell 88PM886 is a PMIC providing several functions such as onkey,
+  regulators or battery and charger.
+
+properties:
+  compatible:
+    const: marvell,88pm886-a1
+
+  reg:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  interrupts:
+    maxItems: 1
+
+  "#interrupt-cells":
+    const: 1
+
+  regulators:
+    $ref: /schemas/regulator/marvell,88pm88x-regulator.yaml#
+
+required:
+  - compatible
+  - reg
+  - interrupt-controller
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      pmic@30 {
+        compatible = "marvell,88pm886-a1";
+        reg = <0x30>;
+        interrupts = <0 4 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-parent = <&gic>;
+        interrupt-controller;
+        #interrupt-cells = <1>;
+
+        regulators {
+          ldo2: ldo2 {
+            regulator-min-microvolt = <3100000>;
+            regulator-max-microvolt = <3300000>;
+            };
+
+          ldo15: ldo15 {
+            regulator-min-microvolt = <3300000>;
+            regulator-max-microvolt = <3300000>;
+            };
+
+          buck2: buck2 {
+            regulator-min-microvolt = <1800000>;
+            regulator-max-microvolt = <1800000>;
+            };
+        };
+      };
+    };
+...
diff --git a/Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml b/Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml
new file mode 100644
index 000000000000..1b4b5f1b4932
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml
@@ -0,0 +1,28 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/marvell,88pm88x-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Regulators of Marvell 88PM88X PMICs.
+
+maintainers:
+  - Karel Balej <balejk@matfyz.cz>
+
+description: |
+  This is a part of device tree bindings for Marvell 88PM88X MFD.
+
+  The regulators node is represented as a sub-node of the PMIC node on the
+  device tree.
+
+  See also Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml for
+  additional information and example.
+
+patternProperties:
+  "^(ldo(1[0-6]|[1-9])|buck[1-5])$":
+    type: object
+    $ref: /schemas/regulator/regulator.yaml#
+    description: LDO or buck regulator.
+    unevaluatedProperties: false
+
+additionalProperties: false
-- 
2.43.0


^ permalink raw reply related

* [RFC PATCH v2 3/6] regulator: add regulators driver for Marvell 88PM886 PMIC
From: Karel Balej @ 2024-02-11  9:35 UTC (permalink / raw)
  To: Karel Balej, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lee Jones, Liam Girdwood, Mark Brown, linux-input,
	devicetree, linux-kernel
  Cc: Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <20240211094609.2223-1-karelb@gimli.ms.mff.cuni.cz>

From: Karel Balej <balejk@matfyz.cz>

Support the LDO and buck regulators of the Marvell 88PM886 PMIC.

88PM886 LDOs match those of 88PM880 which also has several more of them.
88PM880 buck regulators descriptions do not match and they sit on a
different register page and thus need a separate I2C client and regmap.

Signed-off-by: Karel Balej <balejk@matfyz.cz>
---

Notes:
    RFC v2:
    - Drop of_compatible and related code.
    - Drop unused include.
    - Remove some abstraction: use only one regmap for all regulators and
      only mention 88PM886 in Kconfig description.
    - Reword commit message.

 drivers/regulator/88pm88x-regulator.c | 206 ++++++++++++++++++++++++++
 drivers/regulator/Kconfig             |   6 +
 drivers/regulator/Makefile            |   1 +
 3 files changed, 213 insertions(+)
 create mode 100644 drivers/regulator/88pm88x-regulator.c

diff --git a/drivers/regulator/88pm88x-regulator.c b/drivers/regulator/88pm88x-regulator.c
new file mode 100644
index 000000000000..fd84b9604ac6
--- /dev/null
+++ b/drivers/regulator/88pm88x-regulator.c
@@ -0,0 +1,206 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/kernel.h>
+#include <linux/linear_range.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+
+#include <linux/mfd/88pm88x.h>
+
+#define PM88X_REG_LDO_EN1		0x09
+#define PM88X_REG_LDO_EN2		0x0a
+
+#define PM88X_REG_BUCK_EN		0x08
+
+#define PM88X_REG_LDO1_VOUT		0x20
+#define PM88X_REG_LDO2_VOUT		0x26
+#define PM88X_REG_LDO3_VOUT		0x2c
+#define PM88X_REG_LDO4_VOUT		0x32
+#define PM88X_REG_LDO5_VOUT		0x38
+#define PM88X_REG_LDO6_VOUT		0x3e
+#define PM88X_REG_LDO7_VOUT		0x44
+#define PM88X_REG_LDO8_VOUT		0x4a
+#define PM88X_REG_LDO9_VOUT		0x50
+#define PM88X_REG_LDO10_VOUT		0x56
+#define PM88X_REG_LDO11_VOUT		0x5c
+#define PM88X_REG_LDO12_VOUT		0x62
+#define PM88X_REG_LDO13_VOUT		0x68
+#define PM88X_REG_LDO14_VOUT		0x6e
+#define PM88X_REG_LDO15_VOUT		0x74
+#define PM88X_REG_LDO16_VOUT		0x7a
+
+#define PM886_REG_BUCK1_VOUT		0xa5
+#define PM886_REG_BUCK2_VOUT		0xb3
+#define PM886_REG_BUCK3_VOUT		0xc1
+#define PM886_REG_BUCK4_VOUT		0xcf
+#define PM886_REG_BUCK5_VOUT		0xdd
+
+#define PM88X_LDO_VSEL_MASK		0x0f
+#define PM88X_BUCK_VSEL_MASK		0x7f
+
+struct pm88x_regulator {
+	struct regulator_desc desc;
+	int max_uA;
+};
+
+static int pm88x_regulator_get_ilim(struct regulator_dev *rdev)
+{
+	struct pm88x_regulator *data = rdev_get_drvdata(rdev);
+
+	if (!data) {
+		dev_err(&rdev->dev, "Failed to get regulator data\n");
+		return -EINVAL;
+	}
+	return data->max_uA;
+}
+
+static const struct regulator_ops pm88x_ldo_ops = {
+	.list_voltage = regulator_list_voltage_table,
+	.map_voltage = regulator_map_voltage_iterate,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.get_current_limit = pm88x_regulator_get_ilim,
+};
+
+static const struct regulator_ops pm88x_buck_ops = {
+	.list_voltage = regulator_list_voltage_linear_range,
+	.map_voltage = regulator_map_voltage_linear_range,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.get_current_limit = pm88x_regulator_get_ilim,
+};
+
+static const unsigned int pm88x_ldo_volt_table1[] = {
+	1700000, 1800000, 1900000, 2500000, 2800000, 2900000, 3100000, 3300000,
+};
+
+static const unsigned int pm88x_ldo_volt_table2[] = {
+	1200000, 1250000, 1700000, 1800000, 1850000, 1900000, 2500000, 2600000,
+	2700000, 2750000, 2800000, 2850000, 2900000, 3000000, 3100000, 3300000,
+};
+
+static const unsigned int pm88x_ldo_volt_table3[] = {
+	1700000, 1800000, 1900000, 2000000, 2100000, 2500000, 2700000, 2800000,
+};
+
+static const struct linear_range pm88x_buck_volt_ranges1[] = {
+	REGULATOR_LINEAR_RANGE(600000, 0, 79, 12500),
+	REGULATOR_LINEAR_RANGE(1600000, 80, 84, 50000),
+};
+
+static const struct linear_range pm88x_buck_volt_ranges2[] = {
+	REGULATOR_LINEAR_RANGE(600000, 0, 79, 12500),
+	REGULATOR_LINEAR_RANGE(1600000, 80, 114, 50000),
+};
+
+static struct pm88x_regulator pm88x_ldo2 = {
+	.desc = {
+		.name = "LDO2",
+		.id = PM88X_REGULATOR_ID_LDO2,
+		.regulators_node = "regulators",
+		.of_match = "ldo2",
+		.ops = &pm88x_ldo_ops,
+		.type = REGULATOR_VOLTAGE,
+		.enable_reg = PM88X_REG_LDO_EN1,
+		.enable_mask = BIT(1),
+		.volt_table = pm88x_ldo_volt_table1,
+		.n_voltages = ARRAY_SIZE(pm88x_ldo_volt_table1),
+		.vsel_reg = PM88X_REG_LDO2_VOUT,
+		.vsel_mask = PM88X_LDO_VSEL_MASK,
+	},
+	.max_uA = 100000,
+};
+
+static struct pm88x_regulator pm88x_ldo15 = {
+	.desc = {
+		.name = "LDO15",
+		.id = PM88X_REGULATOR_ID_LDO15,
+		.regulators_node = "regulators",
+		.of_match = "ldo15",
+		.ops = &pm88x_ldo_ops,
+		.type = REGULATOR_VOLTAGE,
+		.enable_reg = PM88X_REG_LDO_EN2,
+		.enable_mask = BIT(6),
+		.volt_table = pm88x_ldo_volt_table2,
+		.n_voltages = ARRAY_SIZE(pm88x_ldo_volt_table2),
+		.vsel_reg = PM88X_REG_LDO15_VOUT,
+		.vsel_mask = PM88X_LDO_VSEL_MASK,
+	},
+	.max_uA = 200000,
+};
+
+static struct pm88x_regulator pm886_buck2 = {
+	.desc = {
+		.name = "buck2",
+		.id = PM886_REGULATOR_ID_BUCK2,
+		.regulators_node = "regulators",
+		.of_match = "buck2",
+		.ops = &pm88x_buck_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = 115,
+		.linear_ranges = pm88x_buck_volt_ranges2,
+		.n_linear_ranges = ARRAY_SIZE(pm88x_buck_volt_ranges2),
+		.vsel_reg = PM886_REG_BUCK2_VOUT,
+		.vsel_mask = PM88X_BUCK_VSEL_MASK,
+		.enable_reg = PM88X_REG_BUCK_EN,
+		.enable_mask = BIT(1),
+	},
+	.max_uA = 1200000,
+};
+
+static struct pm88x_regulator *pm88x_regulators[] = {
+	[PM88X_REGULATOR_ID_LDO2] = &pm88x_ldo2,
+	[PM88X_REGULATOR_ID_LDO15] = &pm88x_ldo15,
+	[PM886_REGULATOR_ID_BUCK2] = &pm886_buck2,
+};
+
+static int pm88x_regulator_probe(struct platform_device *pdev)
+{
+	struct pm88x_chip *chip = dev_get_drvdata(pdev->dev.parent);
+	struct regulator_config rcfg = { };
+	struct pm88x_regulator *regulator;
+	struct regulator_desc *rdesc;
+	struct regulator_dev *rdev;
+	int ret;
+
+	if (pdev->id < 0 || pdev->id >= PM88X_REGULATOR_ID_SENTINEL) {
+		dev_err(&pdev->dev, "Invalid regulator ID: %d\n", pdev->id);
+		return -EINVAL;
+	}
+
+	rcfg.dev = pdev->dev.parent;
+	regulator = pm88x_regulators[pdev->id];
+	rdesc = &regulator->desc;
+	rcfg.driver_data = regulator;
+	/* For 88PM886, regmap is the same for LDOs and bucks. */
+	rcfg.regmap = chip->regmaps[PM88X_REGMAP_LDO];
+	rdev = devm_regulator_register(&pdev->dev, rdesc, &rcfg);
+	if (IS_ERR(rdev)) {
+		ret = PTR_ERR(rdev);
+		dev_err(&pdev->dev, "Failed to register %s: %d",
+				rdesc->name, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static struct platform_driver pm88x_regulator_driver = {
+	.driver = {
+		.name = "88pm88x-regulator",
+	},
+	.probe = pm88x_regulator_probe,
+};
+module_platform_driver(pm88x_regulator_driver);
+
+MODULE_DESCRIPTION("Marvell 88PM88X PMIC regulator driver");
+MODULE_AUTHOR("Karel Balej <balejk@matfyz.cz>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 550145f82726..8872d0434412 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -91,6 +91,12 @@ config REGULATOR_88PM8607
 	help
 	  This driver supports 88PM8607 voltage regulator chips.
 
+config REGULATOR_88PM88X
+	tristate "Marvell 88PM886 voltage regulators"
+	depends on MFD_88PM88X_PMIC
+	help
+	  This driver implements support for Marvell 88PM886 voltage regulators.
+
 config REGULATOR_ACT8865
 	tristate "Active-semi act8865 voltage regulator"
 	depends on I2C
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 46fb569e6be8..6cfe9bb7ba2e 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_REGULATOR_USERSPACE_CONSUMER) += userspace-consumer.o
 obj-$(CONFIG_REGULATOR_88PG86X) += 88pg86x.o
 obj-$(CONFIG_REGULATOR_88PM800) += 88pm800-regulator.o
 obj-$(CONFIG_REGULATOR_88PM8607) += 88pm8607.o
+obj-$(CONFIG_REGULATOR_88PM88X) += 88pm88x-regulator.o
 obj-$(CONFIG_REGULATOR_CROS_EC) += cros-ec-regulator.o
 obj-$(CONFIG_REGULATOR_CPCAP) += cpcap-regulator.o
 obj-$(CONFIG_REGULATOR_AAT2870) += aat2870-regulator.o
-- 
2.43.0


^ permalink raw reply related

* [RFC PATCH v2 4/6] dt-bindings: input: add entry for Marvell 88PM88X PMICs onkey
From: Karel Balej @ 2024-02-11  9:35 UTC (permalink / raw)
  To: Karel Balej, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lee Jones, Liam Girdwood, Mark Brown, linux-input,
	devicetree, linux-kernel
  Cc: Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <20240211094609.2223-1-karelb@gimli.ms.mff.cuni.cz>

From: Karel Balej <balejk@matfyz.cz>

Marvell 88PM88X PMICs provide onkey functionality -- add the bindings.

Signed-off-by: Karel Balej <balejk@matfyz.cz>
---

Notes:
    RFC v2:
    - Add wakeup-source property and reference onkey schema from MFD.
    - Reword commit message.

 .../bindings/input/marvell,88pm88x-onkey.yaml | 32 +++++++++++++++++++
 .../bindings/mfd/marvell,88pm88x.yaml         |  8 +++++
 2 files changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml

diff --git a/Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml b/Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml
new file mode 100644
index 000000000000..5d3d451d0e1f
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml
@@ -0,0 +1,32 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/marvell,88pm88x-onkey.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Onkey driver for Marvell 88PM88X PMICs.
+
+maintainers:
+  - Karel Balej <balejk@matfyz.cz>
+
+description: |
+  This module is part of the 88PM88X MFD device. For more details
+  see Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml.
+
+  The onkey controller is represented as a sub-node of the PMIC node in
+  the device tree.
+
+allOf:
+  - $ref: input.yaml#
+
+properties:
+  compatible:
+    const: marvell,88pm88x-onkey
+
+  wakeup-source: true
+
+required:
+  - compatible
+
+additionalProperties: false
+...
diff --git a/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
index 29ab979862d5..2507a73d4dc3 100644
--- a/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
+++ b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
@@ -28,6 +28,9 @@ properties:
   "#interrupt-cells":
     const: 1
 
+  onkey:
+    $ref: /schemas/input/marvell,88pm88x-onkey.yaml
+
   regulators:
     $ref: /schemas/regulator/marvell,88pm88x-regulator.yaml#
 
@@ -53,6 +56,11 @@ examples:
         interrupt-controller;
         #interrupt-cells = <1>;
 
+        onkey {
+          compatible = "marvell,88pm88x-onkey";
+          wakeup-source;
+        };
+
         regulators {
           ldo2: ldo2 {
             regulator-min-microvolt = <3100000>;
-- 
2.43.0


^ permalink raw reply related

* [RFC PATCH v2 5/6] input: add onkey driver for Marvell 88PM88X PMICs
From: Karel Balej @ 2024-02-11  9:35 UTC (permalink / raw)
  To: Karel Balej, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lee Jones, Liam Girdwood, Mark Brown, linux-input,
	devicetree, linux-kernel
  Cc: Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <20240211094609.2223-1-karelb@gimli.ms.mff.cuni.cz>

From: Karel Balej <balejk@matfyz.cz>

Marvell 88PM88X PMICs provide onkey among other things. Add client
driver to handle it. The driver currently only provides a basic support
omitting additional functions found in the vendor version, such as long
onkey and GPIO integration.

Signed-off-by: Karel Balej <balejk@matfyz.cz>
---

Notes:
    RFC v2:
    - Address Dmitry's feedback:
      - Sort includes alphabetically.
      - Drop onkey->irq.
      - ret -> err in irq_handler and no initialization.
      - Break long lines and other formatting.
      - Do not clobber platform_get_irq error.
      - Do not set device parent manually.
      - Use input_set_capability.
      - Use the wakeup-source DT property.
      - Drop of_match_table.
      - Use more temporaries.
      - Use dev_err_probe.
    - Modify Kconfig description.

 drivers/input/misc/88pm88x-onkey.c | 95 ++++++++++++++++++++++++++++++
 drivers/input/misc/Kconfig         |  7 +++
 drivers/input/misc/Makefile        |  1 +
 3 files changed, 103 insertions(+)
 create mode 100644 drivers/input/misc/88pm88x-onkey.c

diff --git a/drivers/input/misc/88pm88x-onkey.c b/drivers/input/misc/88pm88x-onkey.c
new file mode 100644
index 000000000000..2a0bd63a63a7
--- /dev/null
+++ b/drivers/input/misc/88pm88x-onkey.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <linux/mfd/88pm88x.h>
+
+struct pm88x_onkey {
+	struct input_dev *idev;
+	struct pm88x_chip *chip;
+};
+
+static irqreturn_t pm88x_onkey_irq_handler(int irq, void *data)
+{
+	struct pm88x_onkey *onkey = data;
+	struct regmap *regmap = onkey->chip->regmaps[PM88X_REGMAP_BASE];
+	struct input_dev *idev = onkey->idev;
+	struct device *parent = idev->dev.parent;
+	unsigned int val;
+	int err;
+
+	err = regmap_read(regmap, PM88X_REG_STATUS1, &val);
+	if (err) {
+		dev_err(parent, "Failed to read status: %d\n", err);
+		return IRQ_NONE;
+	}
+	val &= PM88X_ONKEY_STS1;
+
+	input_report_key(idev, KEY_POWER, val);
+	input_sync(idev);
+
+	return IRQ_HANDLED;
+}
+
+static int pm88x_onkey_probe(struct platform_device *pdev)
+{
+	struct pm88x_chip *chip = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct pm88x_onkey *onkey;
+	struct input_dev *idev;
+	int irq, err;
+
+	onkey = devm_kzalloc(dev, sizeof(*onkey), GFP_KERNEL);
+	if (!onkey)
+		return -ENOMEM;
+
+	onkey->chip = chip;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return dev_err_probe(dev, irq, "Failed to get IRQ\n");
+
+	idev = devm_input_allocate_device(dev);
+	if (!idev) {
+		dev_err(dev, "Failed to allocate input device\n");
+		return -ENOMEM;
+	}
+	onkey->idev = idev;
+
+	idev->name = "88pm88x-onkey";
+	idev->phys = "88pm88x-onkey/input0";
+	idev->id.bustype = BUS_I2C;
+
+	input_set_capability(idev, EV_KEY, KEY_POWER);
+
+	err = devm_request_threaded_irq(dev, irq, NULL, pm88x_onkey_irq_handler,
+					IRQF_ONESHOT | IRQF_NO_SUSPEND, "onkey",
+					onkey);
+	if (err)
+		return dev_err_probe(dev, err, "Failed to request IRQ\n");
+
+	err = input_register_device(idev);
+	if (err)
+		return dev_err_probe(dev, err, "Failed to register input device\n");
+
+	device_init_wakeup(dev, device_property_read_bool(dev, "wakeup-source"));
+
+	return 0;
+}
+
+static struct platform_driver pm88x_onkey_driver = {
+	.driver = {
+		.name = "88pm88x-onkey",
+	},
+	.probe = pm88x_onkey_probe,
+};
+module_platform_driver(pm88x_onkey_driver);
+
+MODULE_DESCRIPTION("Marvell 88PM88X onkey driver");
+MODULE_AUTHOR("Karel Balej <balejk@matfyz.cz>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 6ba984d7f0b1..97a1ff83e8df 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -33,6 +33,13 @@ config INPUT_88PM80X_ONKEY
 	  To compile this driver as a module, choose M here: the module
 	  will be called 88pm80x_onkey.
 
+config INPUT_88PM88X_ONKEY
+	tristate "Marvell 88PM88X onkey support"
+	depends on MFD_88PM88X_PMIC
+	help
+	  Support the onkey of Marvell 88PM88X PMICs as an input device
+	  reporting power button status.
+
 config INPUT_AB8500_PONKEY
 	tristate "AB8500 Pon (PowerOn) Key"
 	depends on AB8500_CORE
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 04296a4abe8e..eab7a364188c 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -7,6 +7,7 @@
 
 obj-$(CONFIG_INPUT_88PM860X_ONKEY)	+= 88pm860x_onkey.o
 obj-$(CONFIG_INPUT_88PM80X_ONKEY)	+= 88pm80x_onkey.o
+obj-$(CONFIG_INPUT_88PM88X_ONKEY)	+= 88pm88x-onkey.o
 obj-$(CONFIG_INPUT_AB8500_PONKEY)	+= ab8500-ponkey.o
 obj-$(CONFIG_INPUT_AD714X)		+= ad714x.o
 obj-$(CONFIG_INPUT_AD714X_I2C)		+= ad714x-i2c.o
-- 
2.43.0


^ permalink raw reply related

* [RFC PATCH v2 6/6] MAINTAINERS: add myself for Marvell 88PM886 PMIC
From: Karel Balej @ 2024-02-11  9:35 UTC (permalink / raw)
  To: Karel Balej, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lee Jones, Liam Girdwood, Mark Brown, linux-input,
	devicetree, linux-kernel
  Cc: Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <20240211094609.2223-1-karelb@gimli.ms.mff.cuni.cz>

From: Karel Balej <balejk@matfyz.cz>

Add an entry to MAINTAINERS for the Marvell 88PM886 PMIC MFD, onkey and
regulator drivers.

Signed-off-by: Karel Balej <balejk@matfyz.cz>
---

Notes:
    RFC v2:
    - Only mention 88PM886 in the commit message.
    - Add regulator driver.
    - Rename the entry.

 MAINTAINERS | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 960512bec428..c8628b9c633d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12949,6 +12949,17 @@ F:	drivers/net/dsa/mv88e6xxx/
 F:	include/linux/dsa/mv88e6xxx.h
 F:	include/linux/platform_data/mv88e6xxx.h
 
+MARVELL 88PM886 PMIC DRIVER
+M:	Karel Balej <balejk@matfyz.cz>
+S:	Maintained
+F:	Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml
+F:	Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
+F:	Documentation/devicetree/bindings/regulator/marvell,88pm88x-regulator.yaml
+F:	drivers/input/misc/88pm88x-onkey.c
+F:	drivers/mfd/88pm88x.c
+F:	drivers/regulators/88pm88x-regulator.c
+F:	include/linux/mfd/88pm88x.h
+
 MARVELL ARMADA 3700 PHY DRIVERS
 M:	Miquel Raynal <miquel.raynal@bootlin.com>
 S:	Maintained
-- 
2.43.0


^ permalink raw reply related

* [PATCH RESEND v2] HID: input: Add support for micmute LED
From: Bernhard Seibold @ 2024-02-11 10:01 UTC (permalink / raw)
  To: linux-input
  Cc: Jiri Kosina, Benjamin Tissoires, Dmitry Torokhov, Hans de Goede,
	Jamie Lentin, Bernhard Seibold

The USB HID spec describes a number of LEDs that are currently
unsupported. For now, add only the micmute LED since this one is proven
to exist in actual devices. Since LED support via input-leds is
grandfathered, the new LED is added directly in hid-input.

Signed-off-by: Bernhard Seibold <mail@bernhard-seibold.de>
---
 drivers/hid/Kconfig     | 11 +++++
 drivers/hid/hid-input.c | 92 +++++++++++++++++++++++++++++++++++++++++
 include/linux/hid.h     |  1 +
 3 files changed, 104 insertions(+)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 4c682c650704..f8ed13d9740a 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -42,6 +42,17 @@ config HID_BATTERY_STRENGTH
 	that support this feature) through power_supply class so that userspace
 	tools, such as upower, can display it.
 
+config HID_LEDS
+	bool "LED support for HID devices"
+	select LEDS_CLASS
+	default y
+	help
+	This option adds support for LEDs on HID devices. Currently, the
+	only supported LED is microphone mute. For all other LEDs,
+	enable CONFIG_INPUT_LEDS.
+
+	If unsure, say Y.
+
 config HIDRAW
 	bool "/dev/hidraw raw HID device support"
 	help
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index c8b20d44b147..32d3e6a2ac44 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -16,6 +16,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/kernel.h>
+#include <linux/leds.h>
 
 #include <linux/hid.h>
 #include <linux/hid-debug.h>
@@ -104,6 +105,9 @@ static const struct usage_priority hidinput_usages_priorities[] = {
 #define map_key_clear(c)	hid_map_usage_clear(hidinput, usage, &bit, \
 		&max, EV_KEY, (c))
 
+#define setup_led(name, trigger) \
+	hidinput_setup_led(device, field, usage_index, name, trigger)
+
 static bool match_scancode(struct hid_usage *usage,
 			   unsigned int cur_idx, unsigned int scancode)
 {
@@ -674,6 +678,88 @@ static bool hidinput_set_battery_charge_status(struct hid_device *dev,
 }
 #endif	/* CONFIG_HID_BATTERY_STRENGTH */
 
+#ifdef CONFIG_HID_LEDS
+
+struct hid_led {
+	struct list_head list;
+	struct led_classdev cdev;
+	struct hid_field *field;
+	unsigned int offset;
+	char *name;
+};
+
+static int hidinput_led_brightness_set(struct led_classdev *cdev,
+		enum led_brightness value)
+{
+	struct device *dev = cdev->dev->parent;
+	struct hid_device *device = to_hid_device(dev);
+	struct hid_led *led = container_of(cdev, struct hid_led, cdev);
+
+	hid_set_field(led->field, led->offset, !!value);
+	schedule_work(&device->led_work);
+
+	return 0;
+}
+
+static void hidinput_setup_led(struct hid_device *device,
+		struct hid_field *field, unsigned int offset,
+		const char *name, const char *trigger)
+{
+	struct hid_led *led;
+	struct device *dev = &device->dev;
+	struct device *idev = &field->hidinput->input->dev;
+
+	led = kzalloc(sizeof(*led), GFP_KERNEL);
+	if (!led)
+		return;
+
+	led->name = kasprintf(GFP_KERNEL, "%s::%s", dev_name(idev), name);
+	if (!led->name) {
+		kfree(led);
+		return;
+	}
+
+	led->cdev.name = led->name;
+	led->cdev.default_trigger = trigger;
+	led->cdev.max_brightness = 1;
+	led->cdev.brightness_set_blocking = hidinput_led_brightness_set;
+	led->field = field;
+	led->offset = offset;
+
+	if (led_classdev_register(dev, &led->cdev)) {
+		kfree(name);
+		kfree(led);
+		return;
+	}
+
+	list_add_tail(&led->list, &device->leds);
+}
+
+static void hidinput_cleanup_leds(struct hid_device *device)
+{
+	struct hid_led *led, *tmp;
+
+	list_for_each_entry_safe(led, tmp, &device->leds, list) {
+		led_classdev_unregister(&led->cdev);
+		kfree(led->name);
+		kfree(led);
+	}
+}
+
+#else  /* !CONFIG_HID_LEDS */
+
+static void hidinput_setup_led(struct hid_device *device,
+		struct hid_field *field, unsigned int offset,
+		const char *name, const char *trigger)
+{
+}
+
+static void hidinput_cleanup_leds(struct hid_device *device)
+{
+}
+
+#endif  /* CONFIG_HID_LEDS */
+
 static bool hidinput_field_in_collection(struct hid_device *device, struct hid_field *field,
 					 unsigned int type, unsigned int usage)
 {
@@ -935,6 +1021,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 		case 0x19:  map_led (LED_MAIL);     break;    /*   "Message Waiting"          */
 		case 0x4d:  map_led (LED_CHARGING); break;    /*   "External Power Connected" */
 
+		case 0x21:  /* "Microphone" */
+			setup_led("micmute", "audio-micmute");
+			break;
+
 		default: goto ignore;
 		}
 		break;
@@ -2282,6 +2372,7 @@ int hidinput_connect(struct hid_device *hid, unsigned int force)
 	int i, k;
 
 	INIT_LIST_HEAD(&hid->inputs);
+	INIT_LIST_HEAD(&hid->leds);
 	INIT_WORK(&hid->led_work, hidinput_led_worker);
 
 	hid->status &= ~HID_STAT_DUP_DETECTED;
@@ -2380,6 +2471,7 @@ void hidinput_disconnect(struct hid_device *hid)
 {
 	struct hid_input *hidinput, *next;
 
+	hidinput_cleanup_leds(hid);
 	hidinput_cleanup_battery(hid);
 
 	list_for_each_entry_safe(hidinput, next, &hid->inputs, list) {
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 7c26db874ff0..7c0e2789755f 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -617,6 +617,7 @@ struct hid_device {							/* device report descriptor */
 	unsigned country;						/* HID country */
 	struct hid_report_enum report_enum[HID_REPORT_TYPES];
 	struct work_struct led_work;					/* delayed LED worker */
+	struct list_head leds;						/* List of associated LEDs */
 
 	struct semaphore driver_input_lock;				/* protects the current driver */
 	struct device dev;						/* device */
-- 
2.43.0


^ permalink raw reply related

* [PATCH RESEND v3 2/2] Input: add Himax HX852x(ES) touchscreen driver
From: Stephan Gerhold @ 2024-02-11 11:25 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg,
	linux-input, devicetree, linux-kernel, Jeff LaBundy,
	Christophe JAILLET, Jonathan Albrieux, Stephan Gerhold
In-Reply-To: <20240211-hx852x-v3-0-f682e2fcf11d@gerhold.net>

From: Jonathan Albrieux <jonathan.albrieux@gmail.com>

Add a simple driver for the Himax HX852x(ES) touch panel controller,
with support for multi-touch and capacitive touch keys.

The driver is somewhat based on sample code from Himax. However, that
code was so extremely confusing that we spent a significant amount of
time just trying to understand the packet format and register commands.
In this driver they are described with clean structs and defines rather
than lots of magic numbers and offset calculations.

Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
Co-developed-by: Stephan Gerhold <stephan@gerhold.net>
Reviewed-by: Jeff LaBundy <jeff@labundy.com>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 MAINTAINERS                              |   7 +
 drivers/input/touchscreen/Kconfig        |  10 +
 drivers/input/touchscreen/Makefile       |   1 +
 drivers/input/touchscreen/himax_hx852x.c | 500 +++++++++++++++++++++++++++++++
 4 files changed, 518 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0595c832c248..2460736e3b21 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9271,6 +9271,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/input/touchscreen/himax,hx83112b.yaml
 F:	drivers/input/touchscreen/himax_hx83112b.c
 
+HIMAX HX852X TOUCHSCREEN DRIVER
+M:	Stephan Gerhold <stephan@gerhold.net>
+L:	linux-input@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/input/touchscreen/himax,hx852es.yaml
+F:	drivers/input/touchscreen/himax_hx852x.c
+
 HIPPI
 M:	Jes Sorensen <jes@trained-monkey.org>
 L:	linux-hippi@sunsite.dk
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index c821fe3ee794..1ae3113fd4d7 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -458,6 +458,16 @@ config TOUCHSCREEN_HIDEEP
 	  To compile this driver as a module, choose M here : the
 	  module will be called hideep_ts.
 
+config TOUCHSCREEN_HIMAX_HX852X
+	tristate "Himax HX852x(ES) touchscreen"
+	depends on I2C
+	help
+	  Say Y here if you have a Himax HX852x(ES) touchscreen.
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called himax_hx852x.
+
 config TOUCHSCREEN_HYCON_HY46XX
 	tristate "Hycon hy46xx touchscreen support"
 	depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index a81cb5aa21a5..5d7c10858ffb 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_CORE)	+= goodix_berlin_core.o
 obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_I2C)	+= goodix_berlin_i2c.o
 obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_SPI)	+= goodix_berlin_spi.o
 obj-$(CONFIG_TOUCHSCREEN_HIDEEP)	+= hideep.o
+obj-$(CONFIG_TOUCHSCREEN_HIMAX_HX852X)	+= himax_hx852x.o
 obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CSTXXX)	+= hynitron_cstxxx.o
 obj-$(CONFIG_TOUCHSCREEN_ILI210X)	+= ili210x.o
 obj-$(CONFIG_TOUCHSCREEN_ILITEK)	+= ilitek_ts_i2c.o
diff --git a/drivers/input/touchscreen/himax_hx852x.c b/drivers/input/touchscreen/himax_hx852x.c
new file mode 100644
index 000000000000..6aa39f02829d
--- /dev/null
+++ b/drivers/input/touchscreen/himax_hx852x.c
@@ -0,0 +1,500 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Himax HX852x(ES) Touchscreen Driver
+ * Copyright (c) 2020-2023 Stephan Gerhold <stephan@gerhold.net>
+ * Copyright (c) 2020 Jonathan Albrieux <jonathan.albrieux@gmail.com>
+ *
+ * Based on the Himax Android Driver Sample Code Ver 0.3 for HMX852xES chipset:
+ * Copyright (c) 2014 Himax Corporation.
+ */
+
+#include <asm/unaligned.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/input/touchscreen.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regulator/consumer.h>
+
+#define HX852X_COORD_SIZE(fingers)	((fingers) * sizeof(struct hx852x_coord))
+#define HX852X_WIDTH_SIZE(fingers)	ALIGN(fingers, 4)
+#define HX852X_BUF_SIZE(fingers)	(HX852X_COORD_SIZE(fingers) + \
+					 HX852X_WIDTH_SIZE(fingers) + \
+					 sizeof(struct hx852x_touch_info))
+
+#define HX852X_MAX_FINGERS		12
+#define HX852X_MAX_KEY_COUNT		4
+#define HX852X_MAX_BUF_SIZE		HX852X_BUF_SIZE(HX852X_MAX_FINGERS)
+
+#define HX852X_TS_SLEEP_IN		0x80
+#define HX852X_TS_SLEEP_OUT		0x81
+#define HX852X_TS_SENSE_OFF		0x82
+#define HX852X_TS_SENSE_ON		0x83
+#define HX852X_READ_ONE_EVENT		0x85
+#define HX852X_READ_ALL_EVENTS		0x86
+#define HX852X_READ_LATEST_EVENT	0x87
+#define HX852X_CLEAR_EVENT_STACK	0x88
+
+#define HX852X_REG_SRAM_SWITCH		0x8c
+#define HX852X_REG_SRAM_ADDR		0x8b
+#define HX852X_REG_FLASH_RPLACE		0x5a
+
+#define HX852X_SRAM_SWITCH_TEST_MODE	0x14
+#define HX852X_SRAM_ADDR_CONFIG		0x7000
+
+struct hx852x {
+	struct i2c_client *client;
+	struct input_dev *input_dev;
+	struct touchscreen_properties props;
+	struct gpio_desc *reset_gpiod;
+	struct regulator_bulk_data supplies[2];
+	unsigned int max_fingers;
+	unsigned int keycount;
+	unsigned int keycodes[HX852X_MAX_KEY_COUNT];
+};
+
+struct hx852x_config {
+	u8 rx_num;
+	u8 tx_num;
+	u8 max_pt;
+	u8 padding1[3];
+	__be16 x_res;
+	__be16 y_res;
+	u8 padding2[2];
+} __packed __aligned(4);
+
+struct hx852x_coord {
+	__be16 x;
+	__be16 y;
+} __packed __aligned(4);
+
+struct hx852x_touch_info {
+	u8 finger_num;
+	__le16 finger_pressed;
+	u8 padding;
+} __packed __aligned(4);
+
+static int hx852x_i2c_read(struct hx852x *hx, u8 cmd, void *data, u16 len)
+{
+	struct i2c_client *client = hx->client;
+	int ret;
+
+	struct i2c_msg msg[] = {
+		{
+			.addr = client->addr,
+			.flags = 0,
+			.len = 1,
+			.buf = &cmd,
+		},
+		{
+			.addr = client->addr,
+			.flags = I2C_M_RD,
+			.len = len,
+			.buf = data,
+		},
+	};
+
+	ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
+	if (ret != ARRAY_SIZE(msg)) {
+		dev_err(&client->dev, "failed to read %#x: %d\n", cmd, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int hx852x_power_on(struct hx852x *hx)
+{
+	struct device *dev = &hx->client->dev;
+	int error;
+
+	error = regulator_bulk_enable(ARRAY_SIZE(hx->supplies), hx->supplies);
+	if (error) {
+		dev_err(dev, "failed to enable regulators: %d\n", error);
+		return error;
+	}
+
+	gpiod_set_value_cansleep(hx->reset_gpiod, 1);
+	msleep(20);
+	gpiod_set_value_cansleep(hx->reset_gpiod, 0);
+	msleep(20);
+
+	return 0;
+}
+
+static int hx852x_start(struct hx852x *hx)
+{
+	struct device *dev = &hx->client->dev;
+	int error;
+
+	error = i2c_smbus_write_byte(hx->client, HX852X_TS_SLEEP_OUT);
+	if (error) {
+		dev_err(dev, "failed to send TS_SLEEP_OUT: %d\n", error);
+		return error;
+	}
+	msleep(30);
+
+	error = i2c_smbus_write_byte(hx->client, HX852X_TS_SENSE_ON);
+	if (error) {
+		dev_err(dev, "failed to send TS_SENSE_ON: %d\n", error);
+		return error;
+	}
+	msleep(20);
+
+	return 0;
+}
+
+static int hx852x_stop(struct hx852x *hx)
+{
+	struct device *dev = &hx->client->dev;
+	int error;
+
+	error = i2c_smbus_write_byte(hx->client, HX852X_TS_SENSE_OFF);
+	if (error) {
+		dev_err(dev, "failed to send TS_SENSE_OFF: %d\n", error);
+		return error;
+	}
+	msleep(20);
+
+	error = i2c_smbus_write_byte(hx->client, HX852X_TS_SLEEP_IN);
+	if (error) {
+		dev_err(dev, "failed to send TS_SLEEP_IN: %d\n", error);
+		return error;
+	}
+	msleep(30);
+
+	return 0;
+}
+
+static int hx852x_power_off(struct hx852x *hx)
+{
+	struct device *dev = &hx->client->dev;
+	int error;
+
+	error = regulator_bulk_disable(ARRAY_SIZE(hx->supplies), hx->supplies);
+	if (error) {
+		dev_err(dev, "failed to disable regulators: %d\n", error);
+		return error;
+	}
+
+	return 0;
+}
+
+static int hx852x_read_config(struct hx852x *hx)
+{
+	struct device *dev = &hx->client->dev;
+	struct hx852x_config conf;
+	int x_res, y_res;
+	int error;
+
+	error = hx852x_power_on(hx);
+	if (error)
+		return error;
+
+	/* Sensing must be turned on briefly to load the config */
+	error = hx852x_start(hx);
+	if (error)
+		goto err_power_off;
+
+	error = hx852x_stop(hx);
+	if (error)
+		goto err_power_off;
+
+	error = i2c_smbus_write_byte_data(hx->client, HX852X_REG_SRAM_SWITCH,
+					  HX852X_SRAM_SWITCH_TEST_MODE);
+	if (error)
+		goto err_power_off;
+
+	error = i2c_smbus_write_word_data(hx->client, HX852X_REG_SRAM_ADDR,
+					  HX852X_SRAM_ADDR_CONFIG);
+	if (error)
+		goto err_test_mode;
+
+	error = hx852x_i2c_read(hx, HX852X_REG_FLASH_RPLACE, &conf, sizeof(conf));
+	if (error)
+		goto err_test_mode;
+
+	x_res = be16_to_cpu(conf.x_res);
+	y_res = be16_to_cpu(conf.y_res);
+	hx->max_fingers = (conf.max_pt & 0xf0) >> 4;
+	dev_dbg(dev, "x res: %u, y res: %u, max fingers: %u\n",
+		x_res, y_res, hx->max_fingers);
+
+	if (hx->max_fingers > HX852X_MAX_FINGERS) {
+		dev_err(dev, "max supported fingers: %u, found: %u\n",
+			HX852X_MAX_FINGERS, hx->max_fingers);
+		error = -EINVAL;
+		goto err_test_mode;
+	}
+
+	if (x_res && y_res) {
+		input_set_abs_params(hx->input_dev, ABS_MT_POSITION_X, 0, x_res - 1, 0, 0);
+		input_set_abs_params(hx->input_dev, ABS_MT_POSITION_Y, 0, y_res - 1, 0, 0);
+	}
+
+err_test_mode:
+	error = i2c_smbus_write_byte_data(hx->client, HX852X_REG_SRAM_SWITCH, 0) ? : error;
+err_power_off:
+	return hx852x_power_off(hx) ? : error;
+}
+
+static int hx852x_handle_events(struct hx852x *hx)
+{
+	/*
+	 * The event packets have variable size, depending on the amount of
+	 * supported fingers (hx->max_fingers). They are laid out as follows:
+	 *  - struct hx852x_coord[hx->max_fingers]: Coordinates for each finger
+	 *  - u8[ALIGN(hx->max_fingers, 4)]: Touch width for each finger
+	 *      with padding for 32-bit alignment
+	 *  - struct hx852x_touch_info
+	 *
+	 * Load everything into a 32-bit aligned buffer so the coordinates
+	 * can be assigned directly, without using get_unaligned_*().
+	 */
+	u8 buf[HX852X_MAX_BUF_SIZE] __aligned(4);
+	struct hx852x_coord *coord = (struct hx852x_coord *)buf;
+	u8 *width = &buf[HX852X_COORD_SIZE(hx->max_fingers)];
+	struct hx852x_touch_info *info = (struct hx852x_touch_info *)
+		&width[HX852X_WIDTH_SIZE(hx->max_fingers)];
+	unsigned long finger_pressed, key_pressed;
+	unsigned int i, x, y, w;
+	int error;
+
+	error = hx852x_i2c_read(hx, HX852X_READ_ALL_EVENTS, buf,
+				HX852X_BUF_SIZE(hx->max_fingers));
+	if (error)
+		return error;
+
+	finger_pressed = get_unaligned_le16(&info->finger_pressed);
+	key_pressed = finger_pressed >> HX852X_MAX_FINGERS;
+
+	/* All bits are set when no touch is detected */
+	if (info->finger_num == 0xff || !(info->finger_num & 0x0f))
+		finger_pressed = 0;
+	if (key_pressed == 0xf)
+		key_pressed = 0;
+
+	for_each_set_bit(i, &finger_pressed, hx->max_fingers) {
+		x = be16_to_cpu(coord[i].x);
+		y = be16_to_cpu(coord[i].y);
+		w = width[i];
+
+		input_mt_slot(hx->input_dev, i);
+		input_mt_report_slot_state(hx->input_dev, MT_TOOL_FINGER, 1);
+		touchscreen_report_pos(hx->input_dev, &hx->props, x, y, true);
+		input_report_abs(hx->input_dev, ABS_MT_TOUCH_MAJOR, w);
+	}
+	input_mt_sync_frame(hx->input_dev);
+
+	for (i = 0; i < hx->keycount; i++)
+		input_report_key(hx->input_dev, hx->keycodes[i], key_pressed & BIT(i));
+
+	input_sync(hx->input_dev);
+	return 0;
+}
+
+static irqreturn_t hx852x_interrupt(int irq, void *ptr)
+{
+	struct hx852x *hx = ptr;
+	int error;
+
+	error = hx852x_handle_events(hx);
+	if (error) {
+		dev_err_ratelimited(&hx->client->dev, "failed to handle events: %d\n", error);
+		return IRQ_NONE;
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int hx852x_input_open(struct input_dev *dev)
+{
+	struct hx852x *hx = input_get_drvdata(dev);
+	int error;
+
+	error = hx852x_power_on(hx);
+	if (error)
+		return error;
+
+	error = hx852x_start(hx);
+	if (error) {
+		hx852x_power_off(hx);
+		return error;
+	}
+
+	enable_irq(hx->client->irq);
+	return 0;
+}
+
+static void hx852x_input_close(struct input_dev *dev)
+{
+	struct hx852x *hx = input_get_drvdata(dev);
+
+	hx852x_stop(hx);
+	disable_irq(hx->client->irq);
+	hx852x_power_off(hx);
+}
+
+static int hx852x_parse_properties(struct hx852x *hx)
+{
+	struct device *dev = &hx->client->dev;
+	int error, count;
+
+	count = device_property_count_u32(dev, "linux,keycodes");
+	if (count == -EINVAL) {
+		/* Property does not exist, keycodes are optional */
+		return 0;
+	} else if (count < 0) {
+		dev_err(dev, "Failed to read linux,keycodes: %d\n", count);
+		return count;
+	} else if (count > HX852X_MAX_KEY_COUNT) {
+		dev_err(dev, "max supported keys: %u, found: %u\n",
+			HX852X_MAX_KEY_COUNT, hx->keycount);
+		return -EINVAL;
+	}
+	hx->keycount = count;
+
+	error = device_property_read_u32_array(dev, "linux,keycodes",
+					       hx->keycodes, hx->keycount);
+	if (error) {
+		dev_err(dev, "failed to read linux,keycodes: %d\n", error);
+		return error;
+	}
+
+	return 0;
+}
+
+static int hx852x_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct hx852x *hx;
+	int error, i;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C |
+				     I2C_FUNC_SMBUS_WRITE_BYTE |
+				     I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
+				     I2C_FUNC_SMBUS_WRITE_WORD_DATA)) {
+		dev_err(dev, "not all required i2c functionality supported\n");
+		return -ENXIO;
+	}
+
+	hx = devm_kzalloc(dev, sizeof(*hx), GFP_KERNEL);
+	if (!hx)
+		return -ENOMEM;
+
+	hx->client = client;
+	hx->input_dev = devm_input_allocate_device(dev);
+	if (!hx->input_dev)
+		return -ENOMEM;
+
+	hx->input_dev->name = "Himax HX852x";
+	hx->input_dev->id.bustype = BUS_I2C;
+	hx->input_dev->open = hx852x_input_open;
+	hx->input_dev->close = hx852x_input_close;
+
+	i2c_set_clientdata(client, hx);
+	input_set_drvdata(hx->input_dev, hx);
+
+	hx->supplies[0].supply = "vcca";
+	hx->supplies[1].supply = "vccd";
+	error = devm_regulator_bulk_get(dev, ARRAY_SIZE(hx->supplies), hx->supplies);
+	if (error)
+		return dev_err_probe(dev, error, "failed to get regulators\n");
+
+	hx->reset_gpiod = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(hx->reset_gpiod))
+		return dev_err_probe(dev, PTR_ERR(hx->reset_gpiod),
+				     "failed to get reset gpio\n");
+
+	error = devm_request_threaded_irq(dev, client->irq, NULL, hx852x_interrupt,
+					  IRQF_ONESHOT | IRQF_NO_AUTOEN, NULL, hx);
+	if (error)
+		return dev_err_probe(dev, error, "failed to request irq %d", client->irq);
+
+	error = hx852x_read_config(hx);
+	if (error)
+		return error;
+
+	input_set_capability(hx->input_dev, EV_ABS, ABS_MT_POSITION_X);
+	input_set_capability(hx->input_dev, EV_ABS, ABS_MT_POSITION_Y);
+	input_set_abs_params(hx->input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
+
+	touchscreen_parse_properties(hx->input_dev, true, &hx->props);
+	error = hx852x_parse_properties(hx);
+	if (error)
+		return error;
+
+	hx->input_dev->keycode = hx->keycodes;
+	hx->input_dev->keycodemax = hx->keycount;
+	hx->input_dev->keycodesize = sizeof(hx->keycodes[0]);
+	for (i = 0; i < hx->keycount; i++)
+		input_set_capability(hx->input_dev, EV_KEY, hx->keycodes[i]);
+
+	error = input_mt_init_slots(hx->input_dev, hx->max_fingers,
+				    INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
+	if (error)
+		return dev_err_probe(dev, error, "failed to init MT slots\n");
+
+	error = input_register_device(hx->input_dev);
+	if (error)
+		return dev_err_probe(dev, error, "failed to register input device\n");
+
+	return 0;
+}
+
+static int hx852x_suspend(struct device *dev)
+{
+	struct hx852x *hx = dev_get_drvdata(dev);
+	int error = 0;
+
+	mutex_lock(&hx->input_dev->mutex);
+	if (input_device_enabled(hx->input_dev))
+		error = hx852x_stop(hx);
+	mutex_unlock(&hx->input_dev->mutex);
+
+	return error;
+}
+
+static int hx852x_resume(struct device *dev)
+{
+	struct hx852x *hx = dev_get_drvdata(dev);
+	int error = 0;
+
+	mutex_lock(&hx->input_dev->mutex);
+	if (input_device_enabled(hx->input_dev))
+		error = hx852x_start(hx);
+	mutex_unlock(&hx->input_dev->mutex);
+
+	return error;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(hx852x_pm_ops, hx852x_suspend, hx852x_resume);
+
+#ifdef CONFIG_OF
+static const struct of_device_id hx852x_of_match[] = {
+	{ .compatible = "himax,hx852es" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, hx852x_of_match);
+#endif
+
+static struct i2c_driver hx852x_driver = {
+	.probe = hx852x_probe,
+	.driver = {
+		.name = "himax_hx852x",
+		.pm = pm_sleep_ptr(&hx852x_pm_ops),
+		.of_match_table = of_match_ptr(hx852x_of_match),
+	},
+};
+module_i2c_driver(hx852x_driver);
+
+MODULE_DESCRIPTION("Himax HX852x(ES) Touchscreen Driver");
+MODULE_AUTHOR("Jonathan Albrieux <jonathan.albrieux@gmail.com>");
+MODULE_AUTHOR("Stephan Gerhold <stephan@gerhold.net>");
+MODULE_LICENSE("GPL");

-- 
2.43.1


^ permalink raw reply related

* [PATCH RESEND v3 0/2] Input: add Himax HX852x(ES) touchscreen driver
From: Stephan Gerhold @ 2024-02-11 11:25 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg,
	linux-input, devicetree, linux-kernel, Jeff LaBundy,
	Christophe JAILLET, Jonathan Albrieux, Stephan Gerhold,
	Krzysztof Kozlowski

Add DT schema and driver for the Himax HX852x(ES) touch panel 
controller, with support for multi-touch and capacitive touch keys.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Unchanged resend of v3 from Tue, 24 Oct 2023:
https://lore.kernel.org/r/20231024-hx852x-v3-0-a1890d3a81e9@gerhold.net

Changes in v3:
- Fix device_property_count_u32() error handling (Jeff)
- Properly handle errors in hx852x_suspend (Jeff)
- Simplify error handling in hx852x_read_config() (Jeff)
- Close i2c_msg array with trailing comma (Jeff)
- Clean up error handling in hx852x_power_off()
- Link to v2: https://lore.kernel.org/r/20230930-hx852x-v2-0-c5821947b225@gerhold.net

Changes in v2:
- dt-bindings: Swap required:/additionalProperties: (Krzysztof)
- Use dev_err_ratelimited() for error in IRQ thread (Christophe)
- Use dev_err_probe() consistently (Christophe)
- Improve error handling of hx852x_power_off()/hx852x_stop() (Jeff)
- Add linux/of.h and linux/mod_devicetable.h include (Jeff)
- Fix %d -> %u in some format strings (Jeff)
- Fix other small comments from Jeff
- Link to v1: https://lore.kernel.org/r/20230913-hx852x-v1-0-9c1ebff536eb@gerhold.net

---
Jonathan Albrieux (1):
      Input: add Himax HX852x(ES) touchscreen driver

Stephan Gerhold (1):
      dt-bindings: input: touchscreen: document Himax HX852x(ES)

 .../bindings/input/touchscreen/himax,hx852es.yaml  |  81 ++++
 MAINTAINERS                                        |   7 +
 drivers/input/touchscreen/Kconfig                  |  10 +
 drivers/input/touchscreen/Makefile                 |   1 +
 drivers/input/touchscreen/himax_hx852x.c           | 500 +++++++++++++++++++++
 5 files changed, 599 insertions(+)
---
base-commit: d03f030115fe930de1222fef294730ba21b93045
change-id: 20230816-hx852x-3490d2773322

Best regards,
-- 
Stephan Gerhold <stephan@gerhold.net>


^ permalink raw reply

* [PATCH RESEND v3 1/2] dt-bindings: input: touchscreen: document Himax HX852x(ES)
From: Stephan Gerhold @ 2024-02-11 11:25 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg,
	linux-input, devicetree, linux-kernel, Jeff LaBundy,
	Christophe JAILLET, Jonathan Albrieux, Stephan Gerhold,
	Krzysztof Kozlowski
In-Reply-To: <20240211-hx852x-v3-0-f682e2fcf11d@gerhold.net>

Himax HX852x(ES) is a touch panel controller with optional support
for capacitive touch keys.

Unfortunately, the model naming is quite unclear and confusing. There
seems to be a distinction between models (e.g. HX8526) and the "series"
suffix (e.g. -A, -B, -C, -D, -E, -ES). But this doesn't seem to be
applied very consistently because e.g. HX8527-E(44) actually seems to
belong to the -ES series.

The compatible consists of the actual part number followed by the
"series" as fallback compatible. Typically only the latter will be
interesting for drivers as there is no relevant difference on the
driver side.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 .../bindings/input/touchscreen/himax,hx852es.yaml  | 81 ++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/himax,hx852es.yaml b/Documentation/devicetree/bindings/input/touchscreen/himax,hx852es.yaml
new file mode 100644
index 000000000000..40a60880111d
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/himax,hx852es.yaml
@@ -0,0 +1,81 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/himax,hx852es.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Himax HX852x(ES) touch panel controller
+
+maintainers:
+  - Stephan Gerhold <stephan@gerhold.net>
+
+allOf:
+  - $ref: touchscreen.yaml#
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - himax,hx8525e
+          - himax,hx8526e
+          - himax,hx8527e
+      - const: himax,hx852es
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+    description: Touch Screen Interrupt (TSIX), active low
+
+  reset-gpios:
+    maxItems: 1
+    description: External Reset (XRES), active low
+
+  vcca-supply:
+    description: Analog power supply (VCCA)
+
+  vccd-supply:
+    description: Digital power supply (VCCD)
+
+  touchscreen-inverted-x: true
+  touchscreen-inverted-y: true
+  touchscreen-size-x: true
+  touchscreen-size-y: true
+  touchscreen-swapped-x-y: true
+
+  linux,keycodes:
+    minItems: 1
+    maxItems: 4
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - reset-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/input/input.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      touchscreen@48 {
+        compatible = "himax,hx8527e", "himax,hx852es";
+        reg = <0x48>;
+        interrupt-parent = <&tlmm>;
+        interrupts = <13 IRQ_TYPE_LEVEL_LOW>;
+        reset-gpios = <&tlmm 12 GPIO_ACTIVE_LOW>;
+        vcca-supply = <&reg_ts_vcca>;
+        vccd-supply = <&pm8916_l6>;
+        linux,keycodes = <KEY_BACK KEY_HOMEPAGE KEY_APPSELECT>;
+      };
+    };
+
+...

-- 
2.43.1


^ permalink raw reply related

* [PATCH] HID: multitouch: Add required quirk for Synaptics 0xcddc device
From: Manuel Fombuena @ 2024-02-11 19:04 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel
  Cc: Manuel Fombuena

Add support for the pointing stick (Accupoint) and 2 mouse buttons.

Present on some Toshiba/dynabook Portege X30 and X40 laptops.

It should close https://bugzilla.kernel.org/show_bug.cgi?id=205817

Signed-off-by: Manuel Fombuena <fombuena@outlook.com>
---
 drivers/hid/hid-multitouch.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index fd5b0637dad6..3e91e4d6ba6f 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -2151,6 +2151,10 @@ static const struct hid_device_id mt_devices[] = {
 		HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8,
 			USB_VENDOR_ID_SYNAPTICS, 0xcd7e) },
 
+	{ .driver_data = MT_CLS_WIN_8_FORCE_MULTI_INPUT,
+		HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8,
+			USB_VENDOR_ID_SYNAPTICS, 0xcddc) },
+
 	{ .driver_data = MT_CLS_WIN_8_FORCE_MULTI_INPUT,
 		HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8,
 			USB_VENDOR_ID_SYNAPTICS, 0xce08) },
-- 
2.43.0


^ permalink raw reply related

* Re: (subset) [PATCH v2 00/33] spi: get rid of some legacy macros
From: Mark Brown @ 2024-02-12 15:33 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: kernel, Moritz Fischer, Wu Hao, Xu Yilun, Tom Rix, linux-fpga,
	linux-kernel, Alexander Aring, Stefan Schmidt, Miquel Raynal,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	linux-wpan, netdev, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, linux-iio, Dmitry Torokhov, linux-input,
	Ulf Hansson, Rayyan Ansari, Andy Shevchenko, Jonathan Cameron,
	Martin Tuma, Mauro Carvalho Chehab, linux-media, Sergey Kozlov,
	Arnd Bergmann, Greg Kroah-Hartman, Yang Yingliang, linux-mmc,
	Richard Weinberger, Vignesh Raghavendra, Rob Herring,
	Heiko Stuebner, Michal Simek, Amit Kumar Mahapatra via Alsa-devel,
	linux-mtd, Martin Blumenstingl, Geert Uytterhoeven,
	Pali Rohár, Simon Horman, Ronald Wahl, Benson Leung,
	Tzung-Bi Shih, Guenter Roeck, chrome-platform, Max Filippov,
	linux-spi, linux-arm-kernel, Bjorn Andersson, Konrad Dybcio,
	linux-arm-msm, Matthias Brugger, AngeloGioacchino Del Regno,
	linux-mediatek, Thomas Zimmermann, Javier Martinez Canillas,
	Amit Kumar Mahapatra, dri-devel, linux-fbdev, linux-staging,
	Viresh Kumar, Rui Miguel Silva, Johan Hovold, Alex Elder,
	greybus-dev, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe,
	linux-integrity, Herve Codina, Alan Stern, Aaro Koskinen,
	Krzysztof Kozlowski, linux-usb, Helge Deller, Dario Binacchi,
	Kalle Valo, Dmitry Antipov, libertas-dev, linux-wireless,
	Jonathan Corbet, James Clark, Bjorn Helgaas, linux-doc
In-Reply-To: <cover.1705944943.git.u.kleine-koenig@pengutronix.de>

On Mon, 22 Jan 2024 19:06:55 +0100, Uwe Kleine-König wrote:
> this is v2 of this patch set.
> 
> Changes since (implicit) v1, sent with Message-Id:
> cover.1705348269.git.u.kleine-koenig@pengutronix.de:
> 
>  - Rebase to v6.8-rc1
>  - Fix a build failure on sh
>  - Added the tags received in (implicit) v1.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[01/33] fpga: ice40-spi: Follow renaming of SPI "master" to "controller"
        commit: 227ab73b89d66e3064b3c2bcb5fe382b1815763d
[02/33] ieee802154: ca8210: Follow renaming of SPI "master" to "controller"
        commit: 167b78446706bb4d19f7dd93ca320aed25ae1bbd
[03/33] iio: adc: ad_sigma_delta: Follow renaming of SPI "master" to "controller"
        commit: 2780e7b716a605781dbee753ef4983d775a65427
[04/33] Input: pxspad - follow renaming of SPI "master" to "controller"
        commit: a78acec53b8524593afeed7258a442adc3450818
[05/33] Input: synaptics-rmi4 - follow renaming of SPI "master" to "controller"
        commit: 1245633c61baf159fcc1303d7f0855f49831b9c1
[06/33] media: mgb4: Follow renaming of SPI "master" to "controller"
        commit: 2c2f93fbfba7186cc081e23120f169eac3b5b62a
[07/33] media: netup_unidvb: Follow renaming of SPI "master" to "controller"
        commit: cfa13a64bd631d8f04a1c385923706fcef9a63ed
[08/33] media: usb/msi2500: Follow renaming of SPI "master" to "controller"
        commit: dd868ae646d5770f80f90dc056d06eb2e6d39c62
[09/33] media: v4l2-subdev: Follow renaming of SPI "master" to "controller"
        commit: d920b3a672b7f79cd13b341234aebd49233f836c
[10/33] misc: gehc-achc: Follow renaming of SPI "master" to "controller"
        commit: 26dcf09ee5d9ceba2c627ae3ba174a229f25638f
[11/33] mmc: mmc_spi: Follow renaming of SPI "master" to "controller"
        commit: b0a6776e53403aa380411f2a43cdefb9f00ff50a
[12/33] mtd: dataflash: Follow renaming of SPI "master" to "controller"
        commit: 44ee998db9eef84bf005c39486566a67cb018354
[14/33] net: ks8851: Follow renaming of SPI "master" to "controller"
        commit: 1cc711a72ae7fd44e90839f0c8d3226664de55a2
[15/33] net: vertexcom: mse102x: Follow renaming of SPI "master" to "controller"
        commit: 7969b98b80c0332f940c547f84650a20aab33841
[16/33] platform/chrome: cros_ec_spi: Follow renaming of SPI "master" to "controller"
        commit: 85ad0ec049a771c4910c8aebb2d0bd9ce9311fd9
[17/33] spi: bitbang: Follow renaming of SPI "master" to "controller"
        commit: 2259233110d90059187c5ba75537eb93eba8417b
[18/33] spi: cadence-quadspi: Don't emit error message on allocation error
        commit: e71011dacc3413bed4118d2c42f10736ffcd762c
[19/33] spi: cadence-quadspi: Follow renaming of SPI "master" to "controller"
        commit: 28e59d8bf1ace0ddf05f989a48d6824d75731267
[20/33] spi: cavium: Follow renaming of SPI "master" to "controller"
        commit: 1747fbdedba8b6b3fd459895ed5d57e534549884
[21/33] spi: geni-qcom: Follow renaming of SPI "master" to "controller"
        commit: 14cea92338a0776c1615994150e738ac0f5fbb2c
[22/33] spi: loopback-test: Follow renaming of SPI "master" to "controller"
        commit: 2c2310c17fac13aa7e78756d7f3780c7891f9397
[23/33] spi: slave-mt27xx: Follow renaming of SPI "master" to "controller"
        commit: 8197b136bbbe64a7cab1020a4b067020e5977d98
[24/33] spi: spidev: Follow renaming of SPI "master" to "controller"
        commit: d934cd6f0e5d0052772612db4b07df60cb9da387
[25/33] staging: fbtft: Follow renaming of SPI "master" to "controller"
        commit: bbd25d7260eeeaef89f7371cbadcd33dd7f7bff9
[26/33] staging: greybus: spi: Follow renaming of SPI "master" to "controller"
        commit: ee3c668dda3d2783b0fff4091461356fe000e4d8
[27/33] tpm_tis_spi: Follow renaming of SPI "master" to "controller"
        commit: b6af14eacc8814b0986e20507df423cebe9fd859
[28/33] usb: gadget: max3420_udc: Follow renaming of SPI "master" to "controller"
        commit: 8c716f4a3d4fcbec976247e3443d36cbc24c0512
[29/33] video: fbdev: mmp: Follow renaming of SPI "master" to "controller"
        commit: b23031e730e72ec9067b7c38c25e776c5e27e116
[30/33] wifi: libertas: Follow renaming of SPI "master" to "controller"
        commit: 30060d57cee194d6b70283f2faf787e2fdc61b6e
[31/33] spi: fsl-lib: Follow renaming of SPI "master" to "controller"
        commit: 801185efa2402dce57828930e9684884fc8d62da
[32/33] spi: Drop compat layer from renaming "master" to "controller"
        commit: 620d269f29a569ba37419cc03cf1da2d55f6252a
[33/33] Documentation: spi: Update documentation for renaming "master" to "controller"
        commit: 76b31eb4c2da3ddb3195cc14f6aad24908adf524

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark


^ permalink raw reply

* [PATCH 00/10] backlight: Replace struct fb_info in interfaces
From: Thomas Zimmermann @ 2024-02-12 16:16 UTC (permalink / raw)
  To: lee, daniel.thompson, jingoohan1, deller, javierm
  Cc: dri-devel, linux-fbdev, linux-input, linux-pwm, Thomas Zimmermann

Backlight drivers implement struct backlight_ops.check_fb, which
uses struct fb_info in its interface. Replace the callback with one
the does not use fb_info.

In DRM, we have several drivers that implement backlight support. By
including <linux/backlight.h> these drivers depend on <linux/fb.h>.
At the same time, fbdev is deprecated for new drivers and likely to
be replaced on many systems.

This patchset is part of a larger effort to implement the backlight
code without depending on fbdev.

Patch 1 makes the backlight core match backlight and framebuffer
devices via struct fb_info.bl_dev. Patches 2 to 9 then go through
drivers and remove unnecessary implementations of check_fb. Finally,
patch 10 replaces the check_fb hook with controls_device, which
uses the framebuffer's Linux device instead of the framebuffer.

Thomas Zimmermann (10):
  backlight: Match backlight device against struct fb_info.bl_dev
  auxdisplay/ht16k33: Remove struct backlight_ops.check_fb
  hid/hid-picolcd: Fix initialization order
  hid/hid-picolcd: Remove struct backlight_ops.check_fb
  backlight/aat2870-backlight: Remove struct backlight.check_fb
  backlight/pwm-backlight: Remove struct backlight_ops.check_fb
  fbdev/sh_mobile_lcdc_fb: Remove struct backlight_ops.check_fb
  fbdev/ssd1307fb: Init backlight before registering framebuffer
  fbdev/ssd1307fb: Remove struct backlight_ops.check_fb
  backlight: Add controls_device callback to struct backlight_ops

 drivers/auxdisplay/ht16k33.c             |  8 ------
 drivers/hid/hid-picolcd_backlight.c      |  7 ------
 drivers/hid/hid-picolcd_core.c           | 14 +++++------
 drivers/hid/hid-picolcd_fb.c             |  4 +++
 drivers/video/backlight/aat2870_bl.c     |  7 ------
 drivers/video/backlight/backlight.c      |  9 +++++--
 drivers/video/backlight/bd6107.c         | 12 ++++-----
 drivers/video/backlight/gpio_backlight.c | 12 ++++-----
 drivers/video/backlight/lv5207lp.c       | 12 ++++-----
 drivers/video/backlight/pwm_bl.c         | 12 ---------
 drivers/video/fbdev/sh_mobile_lcdcfb.c   |  7 ------
 drivers/video/fbdev/ssd1307fb.c          | 31 +++++++++---------------
 include/linux/backlight.h                | 16 ++++++------
 include/linux/pwm_backlight.h            |  1 -
 14 files changed, 55 insertions(+), 97 deletions(-)

-- 
2.43.0


^ permalink raw reply

* [PATCH 01/10] backlight: Match backlight device against struct fb_info.bl_dev
From: Thomas Zimmermann @ 2024-02-12 16:16 UTC (permalink / raw)
  To: lee, daniel.thompson, jingoohan1, deller, javierm
  Cc: dri-devel, linux-fbdev, linux-input, linux-pwm, Thomas Zimmermann
In-Reply-To: <20240212162645.5661-1-tzimmermann@suse.de>

Framebuffer drivers for devices with dedicated backlight are supposed
to set struct fb_info.bl_dev to the backlight's respective device. Use
the value to match backlight and framebuffer in the backlight core code.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/backlight/backlight.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 86e1cdc8e3697..48844a4f28ad3 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -98,7 +98,8 @@ static int fb_notifier_callback(struct notifier_block *self,
 {
 	struct backlight_device *bd;
 	struct fb_event *evdata = data;
-	int node = evdata->info->node;
+	struct fb_info *info = evdata->info;
+	int node = info->node;
 	int fb_blank = 0;
 
 	/* If we aren't interested in this event, skip it immediately ... */
@@ -110,8 +111,12 @@ static int fb_notifier_callback(struct notifier_block *self,
 
 	if (!bd->ops)
 		goto out;
-	if (bd->ops->check_fb && !bd->ops->check_fb(bd, evdata->info))
+	else if (bd->ops->check_fb && !bd->ops->check_fb(bd, info))
 		goto out;
+#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
+	else if (info->bl_dev && info->bl_dev != bd)
+		goto out;
+#endif
 
 	fb_blank = *(int *)evdata->data;
 	if (fb_blank == FB_BLANK_UNBLANK && !bd->fb_bl_on[node]) {
-- 
2.43.0


^ permalink raw reply related

* [PATCH 02/10] auxdisplay/ht16k33: Remove struct backlight_ops.check_fb
From: Thomas Zimmermann @ 2024-02-12 16:16 UTC (permalink / raw)
  To: lee, daniel.thompson, jingoohan1, deller, javierm
  Cc: dri-devel, linux-fbdev, linux-input, linux-pwm, Thomas Zimmermann,
	Robin van der Gracht
In-Reply-To: <20240212162645.5661-1-tzimmermann@suse.de>

The driver sets struct fb_info.bl_dev to the correct backlight
device. Thus rely on the backlight core code to match backlight
and framebuffer devices, and remove the extra check_fb function
from struct backlight_ops.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Robin van der Gracht <robin@protonic.nl>
---
 drivers/auxdisplay/ht16k33.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
index a90430b7d07ba..0a858db32486b 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -325,16 +325,8 @@ static int ht16k33_bl_update_status(struct backlight_device *bl)
 	return ht16k33_brightness_set(priv, brightness);
 }
 
-static int ht16k33_bl_check_fb(struct backlight_device *bl, struct fb_info *fi)
-{
-	struct ht16k33_priv *priv = bl_get_data(bl);
-
-	return (fi == NULL) || (fi->par == priv);
-}
-
 static const struct backlight_ops ht16k33_bl_ops = {
 	.update_status	= ht16k33_bl_update_status,
-	.check_fb	= ht16k33_bl_check_fb,
 };
 
 /*
-- 
2.43.0


^ permalink raw reply related

* [PATCH 04/10] hid/hid-picolcd: Remove struct backlight_ops.check_fb
From: Thomas Zimmermann @ 2024-02-12 16:16 UTC (permalink / raw)
  To: lee, daniel.thompson, jingoohan1, deller, javierm
  Cc: dri-devel, linux-fbdev, linux-input, linux-pwm, Thomas Zimmermann,
	Bruno Prémont
In-Reply-To: <20240212162645.5661-1-tzimmermann@suse.de>

The driver sets struct fb_info.bl_dev to the correct backlight
device. Thus rely on the backlight core code to match backlight
and framebuffer devices, and remove the extra check_fb function
from struct backlight_ops.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: "Bruno Prémont" <bonbons@linux-vserver.org>
---
 drivers/hid/hid-picolcd_backlight.c | 7 -------
 drivers/hid/hid-picolcd_fb.c        | 4 ++++
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-picolcd_backlight.c b/drivers/hid/hid-picolcd_backlight.c
index 5bd2a8c4bbd66..08d16917eb60b 100644
--- a/drivers/hid/hid-picolcd_backlight.c
+++ b/drivers/hid/hid-picolcd_backlight.c
@@ -9,7 +9,6 @@
 
 #include <linux/hid.h>
 
-#include <linux/fb.h>
 #include <linux/backlight.h>
 
 #include "hid-picolcd.h"
@@ -39,15 +38,9 @@ static int picolcd_set_brightness(struct backlight_device *bdev)
 	return 0;
 }
 
-static int picolcd_check_bl_fb(struct backlight_device *bdev, struct fb_info *fb)
-{
-	return fb && fb == picolcd_fbinfo((struct picolcd_data *)bl_get_data(bdev));
-}
-
 static const struct backlight_ops picolcd_blops = {
 	.update_status  = picolcd_set_brightness,
 	.get_brightness = picolcd_get_brightness,
-	.check_fb       = picolcd_check_bl_fb,
 };
 
 int picolcd_init_backlight(struct picolcd_data *data, struct hid_report *report)
diff --git a/drivers/hid/hid-picolcd_fb.c b/drivers/hid/hid-picolcd_fb.c
index d7dddd99d325e..4500f6e03d32f 100644
--- a/drivers/hid/hid-picolcd_fb.c
+++ b/drivers/hid/hid-picolcd_fb.c
@@ -493,6 +493,10 @@ int picolcd_init_framebuffer(struct picolcd_data *data)
 	info->fix = picolcdfb_fix;
 	info->fix.smem_len   = PICOLCDFB_SIZE*8;
 
+#ifdef CONFIG_HID_PICOLCD_BACKLIGHT
+	info->bl_dev = data->backlight;
+#endif
+
 	fbdata = info->par;
 	spin_lock_init(&fbdata->lock);
 	fbdata->picolcd = data;
-- 
2.43.0


^ permalink raw reply related

* [PATCH 03/10] hid/hid-picolcd: Fix initialization order
From: Thomas Zimmermann @ 2024-02-12 16:16 UTC (permalink / raw)
  To: lee, daniel.thompson, jingoohan1, deller, javierm
  Cc: dri-devel, linux-fbdev, linux-input, linux-pwm, Thomas Zimmermann,
	Bruno Prémont
In-Reply-To: <20240212162645.5661-1-tzimmermann@suse.de>

For drivers that support backlight, LCD and fbdev devices, fbdev has
to be initialized last. See documentation for struct fbinfo.bl_dev.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: "Bruno Prémont" <bonbons@linux-vserver.org>
---
 drivers/hid/hid-picolcd_core.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c
index bbda231a7ce30..5ddebe25eb91f 100644
--- a/drivers/hid/hid-picolcd_core.c
+++ b/drivers/hid/hid-picolcd_core.c
@@ -474,11 +474,6 @@ static int picolcd_probe_lcd(struct hid_device *hdev, struct picolcd_data *data)
 	if (error)
 		goto err;
 
-	/* Set up the framebuffer device */
-	error = picolcd_init_framebuffer(data);
-	if (error)
-		goto err;
-
 	/* Setup lcd class device */
 	error = picolcd_init_lcd(data, picolcd_out_report(REPORT_CONTRAST, hdev));
 	if (error)
@@ -489,6 +484,11 @@ static int picolcd_probe_lcd(struct hid_device *hdev, struct picolcd_data *data)
 	if (error)
 		goto err;
 
+	/* Set up the framebuffer device */
+	error = picolcd_init_framebuffer(data);
+	if (error)
+		goto err;
+
 	/* Setup the LED class devices */
 	error = picolcd_init_leds(data, picolcd_out_report(REPORT_LED_STATE, hdev));
 	if (error)
@@ -502,9 +502,9 @@ static int picolcd_probe_lcd(struct hid_device *hdev, struct picolcd_data *data)
 	return 0;
 err:
 	picolcd_exit_leds(data);
+	picolcd_exit_framebuffer(data);
 	picolcd_exit_backlight(data);
 	picolcd_exit_lcd(data);
-	picolcd_exit_framebuffer(data);
 	picolcd_exit_cir(data);
 	picolcd_exit_keys(data);
 	return error;
@@ -623,9 +623,9 @@ static void picolcd_remove(struct hid_device *hdev)
 	/* Cleanup LED */
 	picolcd_exit_leds(data);
 	/* Clean up the framebuffer */
+	picolcd_exit_framebuffer(data);
 	picolcd_exit_backlight(data);
 	picolcd_exit_lcd(data);
-	picolcd_exit_framebuffer(data);
 	/* Cleanup input */
 	picolcd_exit_cir(data);
 	picolcd_exit_keys(data);
-- 
2.43.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox