[PATCH v2 0/7] Preliminary Platform Driver Support for Lemote Yeeloong Laptops

[PATCH v2 0/7] Preliminary Platform Driver Support for Lemote Yeeloong Laptops

[Yifeng Li]

v2:
 - Fix the missing io.h header for the MFD driver.

 - Use SIMPLE_DEV_PM_OPS instead of the inappropriate .suspend/resume
   in the SCI driver.

Lemote Yeeloong is a laptop powered by Loongson 2F MIPS processor,
primarily a demo platform for hobbyists and developers. It uses an
ENE KB3310B Embedded Controller with customized firmware to implement
hardware and power management.

Due to the lack of separation of responsibilities and functions between
different subsystems, the original platform driver submitted to the
mailing list was a piece of monolithic module that implements numerous
drivers, as a result, it was never accepted.

This patch series was carefully reorganized and partially reimplemented
to avoid those pitfalls in the original driver. This is the first part
of the submission, which modifies the core MIPS code to introduce
preliminary preparation for other subdrivers. Notably, it leverages a
notify chain and MFD cells to avoid introducing unnecessary code into
the core MIPS subsystem.

It features the following...

* Create a MFD driver for KB3310B controller, and move the original
KB3310B controller code from mips/loongson64 to our new MFD driver.
This needs to be reviewed by the MFD subsystem maintainer before the
following can proceed.

* Add yeeloong_sci driver support of System Control Interrupt to
arch/mips/loongson64/lemote-2f. This piece of code is crucial and
has to be included in loongson64/lemote-2f because it reprograms CS5536
southbridge GPIO to handle the underlying interrupts.

* Originally, the SCI code was mixed into the hotkey driver. The new
yeeloong_sci driver was written to avoid introducing unrelated subdriver
code that is unsuitable for Linux/MIPS. It only handle a minimum set of
core operations directly related to the hardware platform, and pass the
events to other subdrivers via a notifier chain. As a result, the SCI
logic and the specific hotkey handling logic have been decoupled.

* Subdrivers - hwmon, battery, backlight, lcd and hotkey are registered
as MFD cells in the MFD driver. It means onlf the MFD driver is resposible
to register the upcoming subdrivers, the core board files mips/loongson64/
will not contain unrelated code.

I've done everything I can think of to factor unrelated code out of the
MIPS subsystem. I hope the refactored code would be acceptable now. Please
review my changeset to see whether it's appropriate.

Thanks,

Yifeng Li (7):
  mfd: yeeloong_kb3310b: support KB3310B EC for Lemote Yeeloong laptops.
  mips: loongson64: select MFD_YEELOONG_KB3310B for LEMOTE_MACH2F.
  mips: loongson64: remove ec_kb3310b.c, use MFD driver.
  mips: loongson64: remove yeeloong_report_lid_status from pm.c
  mips: loongson64: register per-board platform drivers for lemote-2f
  mips: loongson64: Support System Control Interrupts for Lemote
    Yeeloong.
  MAINTAINERS: add myself as a maintainer of MIPS/Loongson2 platform
    code.

 MAINTAINERS                                   |  16 +
 .../include/asm/mach-loongson64/loongson.h    |   3 +
 arch/mips/loongson64/Kconfig                  |   1 +
 arch/mips/loongson64/common/platform.c        |  15 +
 arch/mips/loongson64/lemote-2f/Makefile       |   2 +-
 arch/mips/loongson64/lemote-2f/ec_kb3310b.c   | 129 ------
 arch/mips/loongson64/lemote-2f/ec_kb3310b.h   | 188 ---------
 arch/mips/loongson64/lemote-2f/platform.c     |  47 +++
 arch/mips/loongson64/lemote-2f/pm.c           |  38 +-
 arch/mips/loongson64/lemote-2f/reset.c        |   4 +-
 arch/mips/loongson64/lemote-2f/sci.c          | 394 ++++++++++++++++++
 drivers/mfd/Kconfig                           |  10 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/yeeloong_kb3310b.c                | 207 +++++++++
 include/linux/mfd/yeeloong_kb3310b.h          | 211 ++++++++++
 15 files changed, 916 insertions(+), 350 deletions(-)
 delete mode 100644 arch/mips/loongson64/lemote-2f/ec_kb3310b.c
 delete mode 100644 arch/mips/loongson64/lemote-2f/ec_kb3310b.h
 create mode 100644 arch/mips/loongson64/lemote-2f/platform.c
 create mode 100644 arch/mips/loongson64/lemote-2f/sci.c
 create mode 100644 drivers/mfd/yeeloong_kb3310b.c
 create mode 100644 include/linux/mfd/yeeloong_kb3310b.h

-- 
2.20.1

[PATCH v2 1/7] mfd: yeeloong_kb3310b: support KB3310B EC for Lemote Yeeloong laptops.

[Yifeng Li]

Lemote Yeeloong is a laptop powered by Loongson 2F MIPS processor,
primarily a demo platform for hobbyists and developers. It uses an
ENE KB3310B Embedded Controller with customized firmware to implement
hardware and power management.

A monolithic platform driver code for those functionality has existed
out-of-tree for many years. This commit creates a MFD driver for the EC
chip on Yeeloong laptop to isolate EC-related code from core MIPS code,
and serves as the foundation of various subdrivers.

My original attempt was to create a regmap for subdrivers to access the
EC, unfortunately, the board files in Linux/MIPS still needs to access
the EC directly for power management. Unless we find a better home for
those code, we simply export the EC-related functions.

Signed-off-by: Yifeng Li <tomli@tomli.me>
---
 MAINTAINERS                          |   7 +
 drivers/mfd/Kconfig                  |  10 ++
 drivers/mfd/Makefile                 |   1 +
 drivers/mfd/yeeloong_kb3310b.c       | 207 ++++++++++++++++++++++++++
 include/linux/mfd/yeeloong_kb3310b.h | 211 +++++++++++++++++++++++++++
 5 files changed, 436 insertions(+)
 create mode 100644 drivers/mfd/yeeloong_kb3310b.c
 create mode 100644 include/linux/mfd/yeeloong_kb3310b.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 51029a425dbe..208f19801a23 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16839,6 +16839,13 @@ S:	Maintained
 F:	Documentation/input/devices/yealink.rst
 F:	drivers/input/misc/yealink.*
 
+YEELOONG ENE KB3310B MFD DRIVER
+M:	Tom Li <tomli@tomli.me>
+L:	linux-mips@vger.kernel.org
+S:	Maintained
+F:	drivers/mfd/yeeloong_kb3310b.c
+F:	include/linux/mfd/yeeloong_kb3310b.h
+
 Z8530 DRIVER FOR AX.25
 M:	Joerg Reuter <jreuter@yaina.de>
 W:	http://yaina.de/jreuter/
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index f461460a2aeb..a6da8cce72fc 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1872,6 +1872,16 @@ config MFD_STM32_TIMERS
 	  for PWM and IIO Timer. This driver allow to share the
 	  registers between the others drivers.
 
+config MFD_YEELOONG_KB3310B
+	bool "ENE KB3310B Embedded Controller on Lemote Yeeloong laptops"
+	depends on (MIPS && LEMOTE_MACH2F) || COMPILE_TEST
+	select MFD_CORE
+	help
+          Select this option to enable ENE KB3310B Embedded Controller
+          driver used on Lemote Yeeloong laptops, providing power, battery
+          and backlight services. This is a mandatory dependency for
+          Lemote 2F systems.
+
 menu "Multimedia Capabilities Port drivers"
 	depends on ARCH_SA1100
 
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 12980a4ad460..a3446ce7c384 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -224,6 +224,7 @@ obj-$(CONFIG_MFD_HI655X_PMIC)   += hi655x-pmic.o
 obj-$(CONFIG_MFD_DLN2)		+= dln2.o
 obj-$(CONFIG_MFD_RT5033)	+= rt5033.o
 obj-$(CONFIG_MFD_SKY81452)	+= sky81452.o
+obj-$(CONFIG_MFD_YEELOONG_KB3310B) += yeeloong_kb3310b.o
 
 intel-soc-pmic-objs		:= intel_soc_pmic_core.o intel_soc_pmic_crc.o
 obj-$(CONFIG_INTEL_SOC_PMIC)	+= intel-soc-pmic.o
diff --git a/drivers/mfd/yeeloong_kb3310b.c b/drivers/mfd/yeeloong_kb3310b.c
new file mode 100644
index 000000000000..64d353a83122
--- /dev/null
+++ b/drivers/mfd/yeeloong_kb3310b.c
@@ -0,0 +1,207 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+/*
+ * MFD driver for ENE KB3310B embedded controller on Lemote Yeeloong laptops
+ *
+ * Copyright (C) 2008 Lemote Inc.
+ * Author: liujl <liujl@lemote.com>, 2008-04-20
+ *
+ * Copyright (C) 2018 Yifeng Li
+ * Author: Yifeng Li <tomli@tomli.me>
+ *
+ * This is a MFD driver for the ENE KB3310B Embedded Controller for Lemote
+ * Yeeloong laptops to provide utility functions to access the chip from
+ * subdrivers, and handle events and interrupts in board files. This is a
+ * special-purpose driver, and it's only used on Lemote Yeeloong laptops,
+ * and is a mandatory dependency.
+ *
+ * My original attempt was to create a regmap for subdrivers to access the
+ * EC, unfortunately, the board files in Linux/MIPS still needs to access
+ * the EC directly for power management. Unless we find a better home for
+ * those code, we simply export the EC-related functions.
+ */
+
+#include <linux/export.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/core.h>
+#include <linux/io.h>
+#include <linux/delay.h>
+
+#include <linux/mfd/yeeloong_kb3310b.h>
+
+#define DRV_NAME "yeeloong_kb3310b: "
+
+/*****************************************************************************
+ * Most drivers, such as battery or backlight drivers, uses the I/O ports to
+ * access the Index Registers to obtain hardware status and information from
+ * EC chip.
+ ****************************************************************************/
+static struct kb3310b_chip *kb3310b_fwinfo;
+
+static const struct mfd_cell kb3310b_cells[] = {
+	{
+		.name = "yeeloong_sci"
+	},
+	{
+		.name = "yeeloong_hwmon"
+	},
+	{
+		.name = "yeeloong_battery"
+	},
+	{
+		.name = "yeeloong_backlight"
+	},
+	{
+		.name = "yeeloong_lcd"
+	},
+	{
+		.name = "yeeloong_hotkey"
+	},
+};
+
+static DEFINE_SPINLOCK(kb3310b_index_lock);
+
+u8 kb3310b_read(u16 reg)
+{
+	unsigned long flags;
+	u8 val;
+
+	spin_lock_irqsave(&kb3310b_index_lock, flags);
+
+	outb((reg & 0xff00) >> 8, KB3310B_IO_PORT_HIGH);
+	outb((reg & 0x00ff), KB3310B_IO_PORT_LOW);
+	val = inb(KB3310B_IO_PORT_DATA);
+
+	spin_unlock_irqrestore(&kb3310b_index_lock, flags);
+
+	return val;
+}
+EXPORT_SYMBOL_GPL(kb3310b_read);
+
+void kb3310b_write(u16 reg, u8 val)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&kb3310b_index_lock, flags);
+
+	outb((reg & 0xff00) >> 8, KB3310B_IO_PORT_HIGH);
+	outb((reg & 0x00ff), KB3310B_IO_PORT_LOW);
+	outb(val, KB3310B_IO_PORT_DATA);
+	inb(KB3310B_IO_PORT_DATA);  /* flush pending writes */
+
+	spin_unlock_irqrestore(&kb3310b_index_lock, flags);
+}
+EXPORT_SYMBOL_GPL(kb3310b_write);
+
+bool kb3310b_fw_earlier(char *version)
+{
+	return (strncasecmp(kb3310b_fwinfo->version,
+				version, KB3310B_VERSION_LEN) < 0);
+}
+EXPORT_SYMBOL_GPL(kb3310b_fw_earlier);
+
+static int kb3310b_probe(struct platform_device *pdev)
+{
+	kb3310b_fwinfo = dev_get_platdata(&pdev->dev);
+	pr_info(DRV_NAME "firmware version %s", kb3310b_fwinfo->version);
+
+	return devm_mfd_add_devices(&pdev->dev, -1, kb3310b_cells,
+				    ARRAY_SIZE(kb3310b_cells), NULL, 0, NULL);
+}
+
+static struct platform_driver kb3310b_driver = {
+	.driver = {
+		   .name = "yeeloong_kb3310b",
+	},
+	.probe = kb3310b_probe,
+};
+builtin_platform_driver(kb3310b_driver);
+
+/*****************************************************************************
+ * For interrupt handling and power management, the EC chip is also needed to
+ * be queried from the board file at arch/mips/loongson64, through a separate
+ * command port.
+ *****************************************************************************/
+
+static DEFINE_SPINLOCK(kb3310b_command_lock);
+
+/*
+ * This function is used for EC command writes and corresponding status queries.
+ */
+int kb3310b_query_seq(unsigned char cmd)
+{
+	int timeout;
+	unsigned char status;
+	unsigned long flags;
+
+	spin_lock_irqsave(&kb3310b_command_lock, flags);
+
+	/* make chip goto reset mode */
+	udelay(KB3310B_REG_UDELAY);
+	outb(cmd, KB3310B_CMD_PORT);
+	udelay(KB3310B_REG_UDELAY);
+
+	/* check if the command is received by EC */
+	timeout = KB3310B_CMD_TIMEOUT;
+	status = inb(KB3310B_STS_PORT);
+	while (timeout-- && (status & (1 << 1))) {
+		status = inb(KB3310B_STS_PORT);
+		udelay(KB3310B_REG_UDELAY);
+	}
+
+	spin_unlock_irqrestore(&kb3310b_command_lock, flags);
+
+	if (timeout <= 0) {
+		pr_err(DRV_NAME
+			"(%x/NA) failed to issue command %d, no response!\n",
+			timeout, cmd);
+		return -EINVAL;
+	}
+
+	pr_info(DRV_NAME
+		 "(%x/%x) issued command %d, status: 0x%x\n",
+		 timeout, KB3310B_CMD_TIMEOUT - timeout,
+		 cmd, status);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(kb3310b_query_seq);
+
+/*
+ * Send query command to EC to get the proper event number.
+ */
+int kb3310b_query_event_num(void)
+{
+	return kb3310b_query_seq(KB3310B_CMD_GET_EVENT_NUM);
+}
+EXPORT_SYMBOL_GPL(kb3310b_query_event_num);
+
+/*
+ * Get event number from EC.
+ *
+ * NOTE: This routine must follow the query_event_num function in the
+ * interrupt.
+ */
+int kb3310b_get_event_num(void)
+{
+	int timeout = 100;
+	unsigned char value;
+	unsigned char status;
+
+	udelay(KB3310B_REG_UDELAY);
+	status = inb(KB3310B_STS_PORT);
+	udelay(KB3310B_REG_UDELAY);
+	while (timeout-- && !(status & (1 << 0))) {
+		status = inb(KB3310B_STS_PORT);
+		udelay(KB3310B_REG_UDELAY);
+	}
+	if (timeout <= 0) {
+		pr_info("%s: get event number timeout.\n", __func__);
+		return -EINVAL;
+	}
+	value = inb(KB3310B_DAT_PORT);
+	udelay(KB3310B_REG_UDELAY);
+
+	return value;
+}
+EXPORT_SYMBOL_GPL(kb3310b_get_event_num);
diff --git a/include/linux/mfd/yeeloong_kb3310b.h b/include/linux/mfd/yeeloong_kb3310b.h
new file mode 100644
index 000000000000..1f16ba2579bc
--- /dev/null
+++ b/include/linux/mfd/yeeloong_kb3310b.h
@@ -0,0 +1,211 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+/*
+ * MFD driver for ENE KB3310B embedded controller on Lemote Yeeloong laptops
+ *
+ * Copyright (C) 2008 Lemote Inc.
+ * Author: liujl <liujl@lemote.com>, 2008-04-20
+ *
+ * Copyright (C) 2018 Yifeng Li
+ * Author: Yifeng Li <tomli@tomli.me>
+ */
+
+#ifndef __LINUX_MFD_YEELOONG_KB3310B_H
+#define __LINUX_MFD_YEELOONG_KB3310B_H
+
+extern u8 kb3310b_read(u16 reg);
+extern void kb3310b_write(u16 reg, u8 val);
+extern bool kb3310b_fw_earlier(char *version);
+extern int kb3310b_query_seq(unsigned char cmd);
+extern int kb3310b_query_event_num(void);
+extern int kb3310b_get_event_num(void);
+
+typedef int (*sci_handler) (int status);
+
+extern int yeeloong_sci_register_notify(struct notifier_block *nb);
+extern int yeeloong_sci_unregister_notify(struct notifier_block *nb);
+
+#define KB3310B_VERSION_LEN 8
+
+struct kb3310b_chip {
+	char version[KB3310B_VERSION_LEN];
+};
+
+#define KB3310B_SCI_IRQ_NUM	0x0A
+
+/*
+ * The following registers are determined by the EC index configuration.
+ * 1, fill the PORT_HIGH as EC register high part.
+ * 2, fill the PORT_LOW as EC register low part.
+ * 3, fill the PORT_DATA as EC register write data or get the data from it.
+ */
+#define KB3310B_IO_PORT_HIGH	0x0381
+#define KB3310B_IO_PORT_LOW	0x0382
+#define KB3310B_IO_PORT_DATA	0x0383
+
+/*
+ * EC delay time is 500us for register and status access
+ */
+#define KB3310B_REG_UDELAY	500
+#define KB3310B_CMD_TIMEOUT	0x1000
+
+/*
+ * EC access port for SCI communication
+ */
+#define KB3310B_CMD_PORT		0x66
+#define KB3310B_STS_PORT		0x66
+#define KB3310B_DAT_PORT		0x62
+#define KB3310B_CMD_INIT_IDLE_MODE	0xdd
+#define KB3310B_CMD_EXIT_IDLE_MODE	0xdf
+#define KB3310B_CMD_INIT_RESET_MODE	0xd8
+#define KB3310B_CMD_REBOOT_SYSTEM	0x8c
+#define KB3310B_CMD_GET_EVENT_NUM	0x84
+#define KB3310B_CMD_PROGRAM_PIECE	0xda
+
+/* temperature & fan registers */
+#define KB3310B_REG_TEMPERATURE_VALUE	0xF458
+#define KB3310B_REG_FAN_AUTO_MAN_SWITCH 0xF459
+#define KB3310B_BIT_FAN_AUTO		0
+#define KB3310B_BIT_FAN_MANUAL		1
+#define KB3310B_REG_FAN_CONTROL		0xF4D2
+#define KB3310B_BIT_FAN_CONTROL_ON	(1 << 0)
+#define KB3310B_BIT_FAN_CONTROL_OFF	(0 << 0)
+#define KB3310B_REG_FAN_STATUS		0xF4DA
+#define KB3310B_BIT_FAN_STATUS_ON	(1 << 0)
+#define KB3310B_BIT_FAN_STATUS_OFF	(0 << 0)
+#define KB3310B_REG_FAN_SPEED_HIGH	0xFE22
+#define KB3310B_REG_FAN_SPEED_LOW	0xFE23
+#define KB3310B_REG_FAN_SPEED_LEVEL	0xF4CC
+
+/* fan speed divider */
+#define KB3310B_FAN_SPEED_DIVIDER	480000	/* (60*1000*1000/62.5/2)*/
+
+/* battery registers */
+#define KB3310B_REG_BAT_DESIGN_CAP_HIGH		0xF77D
+#define KB3310B_REG_BAT_DESIGN_CAP_LOW		0xF77E
+#define KB3310B_REG_BAT_FULLCHG_CAP_HIGH	0xF780
+#define KB3310B_REG_BAT_FULLCHG_CAP_LOW		0xF781
+#define KB3310B_REG_BAT_DESIGN_VOL_HIGH		0xF782
+#define KB3310B_REG_BAT_DESIGN_VOL_LOW		0xF783
+#define KB3310B_REG_BAT_CURRENT_HIGH		0xF784
+#define KB3310B_REG_BAT_CURRENT_LOW		0xF785
+#define KB3310B_REG_BAT_VOLTAGE_HIGH		0xF786
+#define KB3310B_REG_BAT_VOLTAGE_LOW		0xF787
+#define KB3310B_REG_BAT_TEMPERATURE_HIGH	0xF788
+#define KB3310B_REG_BAT_TEMPERATURE_LOW		0xF789
+#define KB3310B_REG_BAT_RELATIVE_CAP_HIGH	0xF492
+#define KB3310B_REG_BAT_RELATIVE_CAP_LOW	0xF493
+#define KB3310B_REG_BAT_VENDOR			0xF4C4
+#define KB3310B_FLAG_BAT_VENDOR_SANYO		0x01
+#define KB3310B_FLAG_BAT_VENDOR_SIMPLO		0x02
+#define KB3310B_REG_BAT_CELL_COUNT		0xF4C6
+#define KB3310B_FLAG_BAT_CELL_3S1P		0x03
+#define KB3310B_FLAG_BAT_CELL_3S2P		0x06
+#define KB3310B_REG_BAT_CHARGE			0xF4A2
+#define KB3310B_FLAG_BAT_CHARGE_DISCHARGE	0x01
+#define KB3310B_FLAG_BAT_CHARGE_CHARGE		0x02
+#define KB3310B_FLAG_BAT_CHARGE_ACPOWER		0x00
+#define KB3310B_REG_BAT_STATUS			0xF4B0
+#define KB3310B_BIT_BAT_STATUS_LOW		(1 << 5)
+#define KB3310B_BIT_BAT_STATUS_DESTROY		(1 << 2)
+#define KB3310B_BIT_BAT_STATUS_FULL		(1 << 1)
+#define KB3310B_BIT_BAT_STATUS_IN		(1 << 0)
+#define KB3310B_REG_BAT_CHARGE_STATUS		0xF4B1
+#define KB3310B_BIT_BAT_CHARGE_STATUS_OVERTEMP	(1 << 2)
+#define KB3310B_BIT_BAT_CHARGE_STATUS_PRECHG	(1 << 1)
+#define KB3310B_REG_BAT_STATE			0xF482
+#define KB3310B_BIT_BAT_STATE_CHARGING		(1 << 1)
+#define KB3310B_BIT_BAT_STATE_DISCHARGING	(1 << 0)
+#define KB3310B_REG_BAT_POWER			0xF440
+#define KB3310B_BIT_BAT_POWER_S3		(1 << 2)
+#define KB3310B_BIT_BAT_POWER_ON		(1 << 1)
+#define KB3310B_BIT_BAT_POWER_ACIN		(1 << 0)
+
+/* other registers */
+
+/* Audio: rd/wr */
+#define KB3310B_REG_AUDIO_VOLUME	0xF46C
+#define KB3310B_REG_AUDIO_MUTE		0xF4E7
+#define KB3310B_REG_AUDIO_BEEP		0xF4D0
+
+/* USB port power or not: rd/wr */
+#define KB3310B_REG_USB0_FLAG		0xF461
+#define KB3310B_REG_USB1_FLAG		0xF462
+#define KB3310B_REG_USB2_FLAG		0xF463
+#define KB3310B_BIT_USB_FLAG_ON		1
+#define KB3310B_BIT_USB_FLAG_OFF	0
+
+/* LID */
+#define KB3310B_REG_LID_DETECT		0xF4BD
+#define KB3310B_BIT_LID_DETECT_ON	1
+#define KB3310B_BIT_LID_DETECT_OFF	0
+
+/* CRT */
+#define KB3310B_REG_CRT_DETECT		0xF4AD
+#define KB3310B_BIT_CRT_DETECT_PLUG	1
+#define KB3310B_BIT_CRT_DETECT_UNPLUG	0
+
+/* LCD backlight brightness adjust: 9 levels */
+#define KB3310B_REG_DISPLAY_BRIGHTNESS	0xF4F5
+
+/* Black screen status */
+#define KB3310B_REG_DISPLAY_LCD		0xF79F
+#define KB3310B_BIT_DISPLAY_LCD_ON	1
+#define KB3310B_BIT_DISPLAY_LCD_OFF	0
+
+/* LCD backlight control: off/restore */
+#define KB3310B_REG_BACKLIGHT_CTRL	0xF7BD
+#define KB3310B_BIT_BACKLIGHT_ON	1
+#define KB3310B_BIT_BACKLIGHT_OFF	0
+
+/* Reset the machine auto-clear: rd/wr */
+#define KB3310B_REG_RESET		0xF4EC
+#define KB3310B_BIT_RESET_ON		1
+
+/* Light the LED: rd/wr */
+#define KB3310B_REG_LED			0xF4C8
+#define KB3310B_BIT_LED_RED_POWER	(1 << 0)
+#define KB3310B_BIT_LED_ORANGE_POWER	(1 << 1)
+#define KB3310B_BIT_LED_GREEN_CHARGE	(1 << 2)
+#define KB3310B_BIT_LED_RED_CHARGE	(1 << 3)
+#define KB3310B_BIT_LED_NUMLOCK		(1 << 4)
+
+/* Test LED mode, all LED on/off */
+#define KB3310B_REG_LED_TEST		0xF4C2
+#define KB3310B_BIT_LED_TEST_IN		1
+#define KB3310B_BIT_LED_TEST_OUT	0
+
+/* Camera on/off */
+#define KB3310B_REG_CAMERA_STATUS	0xF46A
+#define KB3310B_BIT_CAMERA_STATUS_ON	1
+#define KB3310B_BIT_CAMERA_STATUS_OFF	0
+#define KB3310B_REG_CAMERA_CONTROL	0xF7B7
+#define KB3310B_BIT_CAMERA_CONTROL_OFF	0
+#define KB3310B_BIT_CAMERA_CONTROL_ON	1
+
+/* WLAN Status */
+#define KB3310B_REG_WLAN		0xF4FA
+#define KB3310B_BIT_WLAN_ON		1
+#define KB3310B_BIT_WLAN_OFF		0
+
+/* SCI Event Number from EC */
+enum {
+	KB3310B_EVENT_START = 0x22,
+	KB3310B_EVENT_LID = 0x23,	   /* LID open/close */
+	KB3310B_EVENT_DISPLAY_TOGGLE,	   /* Fn+F3 for display switch */
+	KB3310B_EVENT_SLEEP,		   /* Fn+F1 for entering sleep mode */
+	KB3310B_EVENT_OVERTEMP,		   /* Over-temperature occurred */
+	KB3310B_EVENT_CRT_DETECT,	   /* CRT is connected */
+	KB3310B_EVENT_CAMERA,		   /* Camera on/off */
+	KB3310B_EVENT_USB_OC2,		   /* USB2 Overcurrent occurred */
+	KB3310B_EVENT_USB_OC0,		   /* USB0 Overcurrent occurred */
+	KB3310B_EVENT_BLACK_SCREEN,	   /* Turn on/off backlight */
+	KB3310B_EVENT_AUDIO_MUTE,	   /* Mute on/off */
+	KB3310B_EVENT_DISPLAY_BRIGHTNESS,  /* LCD backlight brightness adjust */
+	KB3310B_EVENT_AC_BAT,		   /* AC & Battery relative issue */
+	KB3310B_EVENT_AUDIO_VOLUME,	   /* Volume adjust */
+	KB3310B_EVENT_WLAN,		   /* WLAN on/off */
+	KB3310B_EVENT_END
+};
+
+#endif /* !__LINUX_MFD_YEELOONG_KB3310B_H */
-- 
2.20.1

[PATCH v2 2/7] mips: loongson64: select MFD_YEELOONG_KB3310B for LEMOTE_MACH2F.

[Yifeng Li]

To ease the support of platform drivers on Lemote Yeeloong laptop,
code for accessing the embedded controller has been separated from
arch/mips, as a MFD driver. Since the board files here still need
to access the EC directly to handle reboot/shutdown and interrupts,
we make MFD_YEELOONG_KB3310B as a mandatory dependency.

Signed-off-by: Yifeng Li <tomli@tomli.me>
---
 arch/mips/loongson64/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/mips/loongson64/Kconfig b/arch/mips/loongson64/Kconfig
index 4c14a11525f4..b423d5bba812 100644
--- a/arch/mips/loongson64/Kconfig
+++ b/arch/mips/loongson64/Kconfig
@@ -56,6 +56,7 @@ config LEMOTE_MACH2F
 	select SYS_SUPPORTS_HIGHMEM
 	select SYS_SUPPORTS_LITTLE_ENDIAN
 	select LOONGSON_MC146818
+	select MFD_YEELOONG_KB3310B
 	help
 	  Lemote Loongson 2F family machines utilize the 2F revision of
 	  Loongson processor and the AMD CS5536 south bridge.
-- 
2.20.1

[PATCH v2 3/7] mips: loongson64: remove ec_kb3310b.c, use MFD driver.

[Yifeng Li]

We have already converted the supporting code for ENE KB3310B
embedded controller as a separate MFD driver, and select it
as a dependency of LEMOTE_MACH2F.

This commit removes the original implementation of ec_kb3310b.c,
and converts all EC operations to use the utility function provided
by the yeeloong_kb3310b MFD driver instead.

Signed-off-by: Yifeng Li <tomli@tomli.me>
---
 arch/mips/loongson64/lemote-2f/Makefile     |   2 +-
 arch/mips/loongson64/lemote-2f/ec_kb3310b.c | 129 --------------
 arch/mips/loongson64/lemote-2f/ec_kb3310b.h | 188 --------------------
 arch/mips/loongson64/lemote-2f/pm.c         |  18 +-
 arch/mips/loongson64/lemote-2f/reset.c      |   4 +-
 5 files changed, 12 insertions(+), 329 deletions(-)
 delete mode 100644 arch/mips/loongson64/lemote-2f/ec_kb3310b.c
 delete mode 100644 arch/mips/loongson64/lemote-2f/ec_kb3310b.h

diff --git a/arch/mips/loongson64/lemote-2f/Makefile b/arch/mips/loongson64/lemote-2f/Makefile
index b5792c334cd5..ac97f14ea2b7 100644
--- a/arch/mips/loongson64/lemote-2f/Makefile
+++ b/arch/mips/loongson64/lemote-2f/Makefile
@@ -2,7 +2,7 @@
 # Makefile for lemote loongson2f family machines
 #
 
-obj-y += clock.o machtype.o irq.o reset.o dma.o ec_kb3310b.o
+obj-y += clock.o machtype.o irq.o reset.o dma.o
 
 #
 # Suspend Support
diff --git a/arch/mips/loongson64/lemote-2f/ec_kb3310b.c b/arch/mips/loongson64/lemote-2f/ec_kb3310b.c
deleted file mode 100644
index 321822997e76..000000000000
--- a/arch/mips/loongson64/lemote-2f/ec_kb3310b.c
+++ /dev/null
@@ -1,129 +0,0 @@
-/*
- * Basic KB3310B Embedded Controller support for the YeeLoong 2F netbook
- *
- *  Copyright (C) 2008 Lemote Inc.
- *  Author: liujl <liujl@lemote.com>, 2008-04-20
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
-
-#include <linux/io.h>
-#include <linux/export.h>
-#include <linux/spinlock.h>
-#include <linux/delay.h>
-
-#include "ec_kb3310b.h"
-
-static DEFINE_SPINLOCK(index_access_lock);
-static DEFINE_SPINLOCK(port_access_lock);
-
-unsigned char ec_read(unsigned short addr)
-{
-	unsigned char value;
-	unsigned long flags;
-
-	spin_lock_irqsave(&index_access_lock, flags);
-	outb((addr & 0xff00) >> 8, EC_IO_PORT_HIGH);
-	outb((addr & 0x00ff), EC_IO_PORT_LOW);
-	value = inb(EC_IO_PORT_DATA);
-	spin_unlock_irqrestore(&index_access_lock, flags);
-
-	return value;
-}
-EXPORT_SYMBOL_GPL(ec_read);
-
-void ec_write(unsigned short addr, unsigned char val)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&index_access_lock, flags);
-	outb((addr & 0xff00) >> 8, EC_IO_PORT_HIGH);
-	outb((addr & 0x00ff), EC_IO_PORT_LOW);
-	outb(val, EC_IO_PORT_DATA);
-	/*  flush the write action */
-	inb(EC_IO_PORT_DATA);
-	spin_unlock_irqrestore(&index_access_lock, flags);
-}
-EXPORT_SYMBOL_GPL(ec_write);
-
-/*
- * This function is used for EC command writes and corresponding status queries.
- */
-int ec_query_seq(unsigned char cmd)
-{
-	int timeout;
-	unsigned char status;
-	unsigned long flags;
-	int ret = 0;
-
-	spin_lock_irqsave(&port_access_lock, flags);
-
-	/* make chip goto reset mode */
-	udelay(EC_REG_DELAY);
-	outb(cmd, EC_CMD_PORT);
-	udelay(EC_REG_DELAY);
-
-	/* check if the command is received by ec */
-	timeout = EC_CMD_TIMEOUT;
-	status = inb(EC_STS_PORT);
-	while (timeout-- && (status & (1 << 1))) {
-		status = inb(EC_STS_PORT);
-		udelay(EC_REG_DELAY);
-	}
-
-	spin_unlock_irqrestore(&port_access_lock, flags);
-
-	if (timeout <= 0) {
-		printk(KERN_ERR "%s: deadable error : timeout...\n", __func__);
-		ret = -EINVAL;
-	} else
-		printk(KERN_INFO
-			   "(%x/%d)ec issued command %d status : 0x%x\n",
-			   timeout, EC_CMD_TIMEOUT - timeout, cmd, status);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(ec_query_seq);
-
-/*
- * Send query command to EC to get the proper event number
- */
-int ec_query_event_num(void)
-{
-	return ec_query_seq(CMD_GET_EVENT_NUM);
-}
-EXPORT_SYMBOL(ec_query_event_num);
-
-/*
- * Get event number from EC
- *
- * NOTE: This routine must follow the query_event_num function in the
- * interrupt.
- */
-int ec_get_event_num(void)
-{
-	int timeout = 100;
-	unsigned char value;
-	unsigned char status;
-
-	udelay(EC_REG_DELAY);
-	status = inb(EC_STS_PORT);
-	udelay(EC_REG_DELAY);
-	while (timeout-- && !(status & (1 << 0))) {
-		status = inb(EC_STS_PORT);
-		udelay(EC_REG_DELAY);
-	}
-	if (timeout <= 0) {
-		pr_info("%s: get event number timeout.\n", __func__);
-
-		return -EINVAL;
-	}
-	value = inb(EC_DAT_PORT);
-	udelay(EC_REG_DELAY);
-
-	return value;
-}
-EXPORT_SYMBOL(ec_get_event_num);
diff --git a/arch/mips/loongson64/lemote-2f/ec_kb3310b.h b/arch/mips/loongson64/lemote-2f/ec_kb3310b.h
deleted file mode 100644
index 5a3f1860d4d2..000000000000
--- a/arch/mips/loongson64/lemote-2f/ec_kb3310b.h
+++ /dev/null
@@ -1,188 +0,0 @@
-/*
- * KB3310B Embedded Controller
- *
- *  Copyright (C) 2008 Lemote Inc.
- *  Author: liujl <liujl@lemote.com>, 2008-03-14
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
-
-#ifndef _EC_KB3310B_H
-#define _EC_KB3310B_H
-
-extern unsigned char ec_read(unsigned short addr);
-extern void ec_write(unsigned short addr, unsigned char val);
-extern int ec_query_seq(unsigned char cmd);
-extern int ec_query_event_num(void);
-extern int ec_get_event_num(void);
-
-typedef int (*sci_handler) (int status);
-extern sci_handler yeeloong_report_lid_status;
-
-#define SCI_IRQ_NUM 0x0A
-
-/*
- * The following registers are determined by the EC index configuration.
- * 1, fill the PORT_HIGH as EC register high part.
- * 2, fill the PORT_LOW as EC register low part.
- * 3, fill the PORT_DATA as EC register write data or get the data from it.
- */
-#define EC_IO_PORT_HIGH 0x0381
-#define EC_IO_PORT_LOW	0x0382
-#define EC_IO_PORT_DATA 0x0383
-
-/*
- * EC delay time is 500us for register and status access
- */
-#define EC_REG_DELAY	500	/* unit : us */
-#define EC_CMD_TIMEOUT	0x1000
-
-/*
- * EC access port for SCI communication
- */
-#define EC_CMD_PORT		0x66
-#define EC_STS_PORT		0x66
-#define EC_DAT_PORT		0x62
-#define CMD_INIT_IDLE_MODE	0xdd
-#define CMD_EXIT_IDLE_MODE	0xdf
-#define CMD_INIT_RESET_MODE	0xd8
-#define CMD_REBOOT_SYSTEM	0x8c
-#define CMD_GET_EVENT_NUM	0x84
-#define CMD_PROGRAM_PIECE	0xda
-
-/* temperature & fan registers */
-#define REG_TEMPERATURE_VALUE	0xF458
-#define REG_FAN_AUTO_MAN_SWITCH 0xF459
-#define BIT_FAN_AUTO		0
-#define BIT_FAN_MANUAL		1
-#define REG_FAN_CONTROL		0xF4D2
-#define BIT_FAN_CONTROL_ON	(1 << 0)
-#define BIT_FAN_CONTROL_OFF	(0 << 0)
-#define REG_FAN_STATUS		0xF4DA
-#define BIT_FAN_STATUS_ON	(1 << 0)
-#define BIT_FAN_STATUS_OFF	(0 << 0)
-#define REG_FAN_SPEED_HIGH	0xFE22
-#define REG_FAN_SPEED_LOW	0xFE23
-#define REG_FAN_SPEED_LEVEL	0xF4CC
-/* fan speed divider */
-#define FAN_SPEED_DIVIDER	480000	/* (60*1000*1000/62.5/2)*/
-
-/* battery registers */
-#define REG_BAT_DESIGN_CAP_HIGH		0xF77D
-#define REG_BAT_DESIGN_CAP_LOW		0xF77E
-#define REG_BAT_FULLCHG_CAP_HIGH	0xF780
-#define REG_BAT_FULLCHG_CAP_LOW		0xF781
-#define REG_BAT_DESIGN_VOL_HIGH		0xF782
-#define REG_BAT_DESIGN_VOL_LOW		0xF783
-#define REG_BAT_CURRENT_HIGH		0xF784
-#define REG_BAT_CURRENT_LOW		0xF785
-#define REG_BAT_VOLTAGE_HIGH		0xF786
-#define REG_BAT_VOLTAGE_LOW		0xF787
-#define REG_BAT_TEMPERATURE_HIGH	0xF788
-#define REG_BAT_TEMPERATURE_LOW		0xF789
-#define REG_BAT_RELATIVE_CAP_HIGH	0xF492
-#define REG_BAT_RELATIVE_CAP_LOW	0xF493
-#define REG_BAT_VENDOR			0xF4C4
-#define FLAG_BAT_VENDOR_SANYO		0x01
-#define FLAG_BAT_VENDOR_SIMPLO		0x02
-#define REG_BAT_CELL_COUNT		0xF4C6
-#define FLAG_BAT_CELL_3S1P		0x03
-#define FLAG_BAT_CELL_3S2P		0x06
-#define REG_BAT_CHARGE			0xF4A2
-#define FLAG_BAT_CHARGE_DISCHARGE	0x01
-#define FLAG_BAT_CHARGE_CHARGE		0x02
-#define FLAG_BAT_CHARGE_ACPOWER		0x00
-#define REG_BAT_STATUS			0xF4B0
-#define BIT_BAT_STATUS_LOW		(1 << 5)
-#define BIT_BAT_STATUS_DESTROY		(1 << 2)
-#define BIT_BAT_STATUS_FULL		(1 << 1)
-#define BIT_BAT_STATUS_IN		(1 << 0)
-#define REG_BAT_CHARGE_STATUS		0xF4B1
-#define BIT_BAT_CHARGE_STATUS_OVERTEMP	(1 << 2)
-#define BIT_BAT_CHARGE_STATUS_PRECHG	(1 << 1)
-#define REG_BAT_STATE			0xF482
-#define BIT_BAT_STATE_CHARGING		(1 << 1)
-#define BIT_BAT_STATE_DISCHARGING	(1 << 0)
-#define REG_BAT_POWER			0xF440
-#define BIT_BAT_POWER_S3		(1 << 2)
-#define BIT_BAT_POWER_ON		(1 << 1)
-#define BIT_BAT_POWER_ACIN		(1 << 0)
-
-/* other registers */
-/* Audio: rd/wr */
-#define REG_AUDIO_VOLUME	0xF46C
-#define REG_AUDIO_MUTE		0xF4E7
-#define REG_AUDIO_BEEP		0xF4D0
-/* USB port power or not: rd/wr */
-#define REG_USB0_FLAG		0xF461
-#define REG_USB1_FLAG		0xF462
-#define REG_USB2_FLAG		0xF463
-#define BIT_USB_FLAG_ON		1
-#define BIT_USB_FLAG_OFF	0
-/* LID */
-#define REG_LID_DETECT		0xF4BD
-#define BIT_LID_DETECT_ON	1
-#define BIT_LID_DETECT_OFF	0
-/* CRT */
-#define REG_CRT_DETECT		0xF4AD
-#define BIT_CRT_DETECT_PLUG	1
-#define BIT_CRT_DETECT_UNPLUG	0
-/* LCD backlight brightness adjust: 9 levels */
-#define REG_DISPLAY_BRIGHTNESS	0xF4F5
-/* Black screen Status */
-#define BIT_DISPLAY_LCD_ON	1
-#define BIT_DISPLAY_LCD_OFF	0
-/* LCD backlight control: off/restore */
-#define REG_BACKLIGHT_CTRL	0xF7BD
-#define BIT_BACKLIGHT_ON	1
-#define BIT_BACKLIGHT_OFF	0
-/* Reset the machine auto-clear: rd/wr */
-#define REG_RESET		0xF4EC
-#define BIT_RESET_ON		1
-/* Light the led: rd/wr */
-#define REG_LED			0xF4C8
-#define BIT_LED_RED_POWER	(1 << 0)
-#define BIT_LED_ORANGE_POWER	(1 << 1)
-#define BIT_LED_GREEN_CHARGE	(1 << 2)
-#define BIT_LED_RED_CHARGE	(1 << 3)
-#define BIT_LED_NUMLOCK		(1 << 4)
-/* Test led mode, all led on/off */
-#define REG_LED_TEST		0xF4C2
-#define BIT_LED_TEST_IN		1
-#define BIT_LED_TEST_OUT	0
-/* Camera on/off */
-#define REG_CAMERA_STATUS	0xF46A
-#define BIT_CAMERA_STATUS_ON	1
-#define BIT_CAMERA_STATUS_OFF	0
-#define REG_CAMERA_CONTROL	0xF7B7
-#define BIT_CAMERA_CONTROL_OFF	0
-#define BIT_CAMERA_CONTROL_ON	1
-/* Wlan Status */
-#define REG_WLAN		0xF4FA
-#define BIT_WLAN_ON		1
-#define BIT_WLAN_OFF		0
-#define REG_DISPLAY_LCD		0xF79F
-
-/* SCI Event Number from EC */
-enum {
-	EVENT_LID = 0x23,	/*  LID open/close */
-	EVENT_DISPLAY_TOGGLE,	/*  Fn+F3 for display switch */
-	EVENT_SLEEP,		/*  Fn+F1 for entering sleep mode */
-	EVENT_OVERTEMP,		/*  Over-temperature happened */
-	EVENT_CRT_DETECT,	/*  CRT is connected */
-	EVENT_CAMERA,		/*  Camera on/off */
-	EVENT_USB_OC2,		/*  USB2 Over Current occurred */
-	EVENT_USB_OC0,		/*  USB0 Over Current occurred */
-	EVENT_BLACK_SCREEN,	/*  Turn on/off backlight */
-	EVENT_AUDIO_MUTE,	/*  Mute on/off */
-	EVENT_DISPLAY_BRIGHTNESS,/* LCD backlight brightness adjust */
-	EVENT_AC_BAT,		/*  AC & Battery relative issue */
-	EVENT_AUDIO_VOLUME,	/*  Volume adjust */
-	EVENT_WLAN,		/*  Wlan on/off */
-	EVENT_END
-};
-
-#endif /* !_EC_KB3310B_H */
diff --git a/arch/mips/loongson64/lemote-2f/pm.c b/arch/mips/loongson64/lemote-2f/pm.c
index 6859e934862d..4ee7e9864700 100644
--- a/arch/mips/loongson64/lemote-2f/pm.c
+++ b/arch/mips/loongson64/lemote-2f/pm.c
@@ -23,7 +23,7 @@
 #include <loongson.h>
 
 #include <cs5536/cs5536_mfgpt.h>
-#include "ec_kb3310b.h"
+#include <linux/mfd/yeeloong_kb3310b.h>
 
 #define I8042_KBD_IRQ		1
 #define I8042_CTR_KBDINT	0x01
@@ -70,7 +70,7 @@ void setup_wakeup_events(void)
 		/* Wakeup CPU via SCI lid open event */
 		outb(irq_mask & ~(1 << PIC_CASCADE_IR), PIC_MASTER_IMR);
 		inb(PIC_MASTER_IMR);
-		outb(0xff & ~(1 << (SCI_IRQ_NUM - 8)), PIC_SLAVE_IMR);
+		outb(0xff & ~(1 << (KB3310B_SCI_IRQ_NUM - 8)), PIC_SLAVE_IMR);
 		inb(PIC_SLAVE_IMR);
 
 		break;
@@ -88,7 +88,7 @@ EXPORT_SYMBOL(yeeloong_report_lid_status);
 static void yeeloong_lid_update_task(struct work_struct *work)
 {
 	if (yeeloong_report_lid_status)
-		yeeloong_report_lid_status(BIT_LID_DETECT_ON);
+		yeeloong_report_lid_status(KB3310B_BIT_LID_DETECT_ON);
 }
 
 int wakeup_loongson(void)
@@ -104,21 +104,21 @@ int wakeup_loongson(void)
 
 	if (irq == I8042_KBD_IRQ)
 		return 1;
-	else if (irq == SCI_IRQ_NUM) {
+	else if (irq == KB3310B_SCI_IRQ_NUM) {
 		int ret, sci_event;
 		/* query the event number */
-		ret = ec_query_seq(CMD_GET_EVENT_NUM);
+		ret = kb3310b_query_seq(KB3310B_CMD_GET_EVENT_NUM);
 		if (ret < 0)
 			return 0;
-		sci_event = ec_get_event_num();
+		sci_event = kb3310b_get_event_num();
 		if (sci_event < 0)
 			return 0;
-		if (sci_event == EVENT_LID) {
+		if (sci_event == KB3310B_EVENT_LID) {
 			int lid_status;
 			/* check the LID status */
-			lid_status = ec_read(REG_LID_DETECT);
+			lid_status = kb3310b_read(KB3310B_REG_LID_DETECT);
 			/* wakeup cpu when people open the LID */
-			if (lid_status == BIT_LID_DETECT_ON) {
+			if (lid_status == KB3310B_BIT_LID_DETECT_ON) {
 				/* If we call it directly here, the WARNING
 				 * will be sent out by getnstimeofday
 				 * via "WARN_ON(timekeeping_suspended);"
diff --git a/arch/mips/loongson64/lemote-2f/reset.c b/arch/mips/loongson64/lemote-2f/reset.c
index a26ca7fcd7e0..c5e2afbb8121 100644
--- a/arch/mips/loongson64/lemote-2f/reset.c
+++ b/arch/mips/loongson64/lemote-2f/reset.c
@@ -20,7 +20,7 @@
 #include <loongson.h>
 
 #include <cs5536/cs5536.h>
-#include "ec_kb3310b.h"
+#include <linux/mfd/yeeloong_kb3310b.h>
 
 static void reset_cpu(void)
 {
@@ -81,7 +81,7 @@ static void ml2f_reboot(void)
 	reset_cpu();
 
 	/* sending an reset signal to EC(embedded controller) */
-	ec_write(REG_RESET, BIT_RESET_ON);
+	kb3310b_write(KB3310B_REG_RESET, KB3310B_BIT_RESET_ON);
 }
 
 #define yl2f89_reboot ml2f_reboot
-- 
2.20.1

[PATCH v2 4/7] mips: loongson64: remove yeeloong_report_lid_status from pm.c

[Yifeng Li]

There is some complicated logic in lemote-2f/pm.c. During wakeup, it
creates a delayed_work to execute a callback to the function
yeeloong_report_lid_status(). It's only purpose is to report the current
status of the laptop lid switch, and this callback function wan not
implemented in the mainline kernel.

This level of overenginnering hardly makes sense. All we need is to report
the laptop lid switch unconditionally upon wakeup in the future PM code,
which is being worked on.

Signed-off-by: Yifeng Li <tomli@tomli.me>
---
 arch/mips/loongson64/lemote-2f/pm.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/arch/mips/loongson64/lemote-2f/pm.c b/arch/mips/loongson64/lemote-2f/pm.c
index 4ee7e9864700..ebe4b57535f0 100644
--- a/arch/mips/loongson64/lemote-2f/pm.c
+++ b/arch/mips/loongson64/lemote-2f/pm.c
@@ -80,17 +80,6 @@ void setup_wakeup_events(void)
 	}
 }
 
-static struct delayed_work lid_task;
-static int initialized;
-/* yeeloong_report_lid_status will be implemented in yeeloong_laptop.c */
-sci_handler yeeloong_report_lid_status;
-EXPORT_SYMBOL(yeeloong_report_lid_status);
-static void yeeloong_lid_update_task(struct work_struct *work)
-{
-	if (yeeloong_report_lid_status)
-		yeeloong_report_lid_status(KB3310B_BIT_LID_DETECT_ON);
-}
-
 int wakeup_loongson(void)
 {
 	int irq;
@@ -119,17 +108,6 @@ int wakeup_loongson(void)
 			lid_status = kb3310b_read(KB3310B_REG_LID_DETECT);
 			/* wakeup cpu when people open the LID */
 			if (lid_status == KB3310B_BIT_LID_DETECT_ON) {
-				/* If we call it directly here, the WARNING
-				 * will be sent out by getnstimeofday
-				 * via "WARN_ON(timekeeping_suspended);"
-				 * because we can not schedule in suspend mode.
-				 */
-				if (initialized == 0) {
-					INIT_DELAYED_WORK(&lid_task,
-						yeeloong_lid_update_task);
-					initialized = 1;
-				}
-				schedule_delayed_work(&lid_task, 1);
 				return 1;
 			}
 		}
-- 
2.20.1

[PATCH v2 5/7] mips: loongson64: register per-board platform drivers for lemote-2f

[Yifeng Li]

Currently, common/platform.c registers the Loongson 2F cpufreq driver
during boot time for all boards. To support platform drivers for Lemote
Yeeloong laptops, we need to register more drivers.

First, we add support for per-board platform drivers. Just like how IRQ,
DMA, or reset logic is implemented for each board, we introduces a call
of mach_platform_init() in common/platform.c, to allow each board to have
its own platform.c to register platform drivers.

Then, we implement lemote-2f/platform.c to register the MFD driver for
Yeeloong laptops.

So far, only one board, lemote-f2, is using this facility, so we hardcode
the call of mach_platform_init() in common/platform.c as a ifdef for now.

Signed-off-by: Yifeng Li <tomli@tomli.me>
---
 .../include/asm/mach-loongson64/loongson.h    |  3 ++
 arch/mips/loongson64/common/platform.c        | 15 ++++++
 arch/mips/loongson64/lemote-2f/Makefile       |  2 +-
 arch/mips/loongson64/lemote-2f/platform.c     | 47 +++++++++++++++++++
 4 files changed, 66 insertions(+), 1 deletion(-)
 create mode 100644 arch/mips/loongson64/lemote-2f/platform.c

diff --git a/arch/mips/include/asm/mach-loongson64/loongson.h b/arch/mips/include/asm/mach-loongson64/loongson.h
index b6870fec0f99..0ea43479d9f8 100644
--- a/arch/mips/include/asm/mach-loongson64/loongson.h
+++ b/arch/mips/include/asm/mach-loongson64/loongson.h
@@ -23,6 +23,9 @@ extern void bonito_irq_init(void);
 extern void mach_prepare_reboot(void);
 extern void mach_prepare_shutdown(void);
 
+/* machine-specific platform driver registration */
+extern int mach_platform_init(void) __init;
+
 /* environment arguments from bootloader */
 extern u32 cpu_clock_freq;
 extern u32 memsize, highmemsize;
diff --git a/arch/mips/loongson64/common/platform.c b/arch/mips/loongson64/common/platform.c
index 0ed38321a9a2..f8a205bae5da 100644
--- a/arch/mips/loongson64/common/platform.c
+++ b/arch/mips/loongson64/common/platform.c
@@ -11,6 +11,7 @@
 #include <linux/err.h>
 #include <linux/smp.h>
 #include <linux/platform_device.h>
+#include <loongson.h>
 
 static struct platform_device loongson2_cpufreq_device = {
 	.name = "loongson2_cpufreq",
@@ -29,3 +30,17 @@ static int __init loongson2_cpufreq_init(void)
 }
 
 arch_initcall(loongson2_cpufreq_init);
+
+/*
+ * Currently, only LEMOTE_MACH2F implements mach_platform_init();
+ * Fuloong-2E or Loongson3 does not have platform drivers to register
+ * at here yet.
+ */
+#ifdef CONFIG_LEMOTE_MACH2F
+static int __init loongson2_platform_init(void)
+{
+	return mach_platform_init();
+}
+
+device_initcall(loongson2_platform_init);
+#endif
diff --git a/arch/mips/loongson64/lemote-2f/Makefile b/arch/mips/loongson64/lemote-2f/Makefile
index ac97f14ea2b7..2b18752424ee 100644
--- a/arch/mips/loongson64/lemote-2f/Makefile
+++ b/arch/mips/loongson64/lemote-2f/Makefile
@@ -2,7 +2,7 @@
 # Makefile for lemote loongson2f family machines
 #
 
-obj-y += clock.o machtype.o irq.o reset.o dma.o
+obj-y += clock.o machtype.o irq.o reset.o dma.o platform.o
 
 #
 # Suspend Support
diff --git a/arch/mips/loongson64/lemote-2f/platform.c b/arch/mips/loongson64/lemote-2f/platform.c
new file mode 100644
index 000000000000..c8a8c597e384
--- /dev/null
+++ b/arch/mips/loongson64/lemote-2f/platform.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+/*
+ * Copyright (C) 2009 Lemote Inc.
+ * Author: Wu Zhangjin, wuzhangjin@gmail.com
+ *
+ * Copyright (C) 2019 Yifeng Li
+ * Author: Yifeng Li <tomli@tomli.me>
+ */
+
+#include <asm/bootinfo.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/yeeloong_kb3310b.h>
+
+static struct kb3310b_chip yeeloong_ec_info;
+
+static struct platform_device yeeloong_ec_device = {
+	.name = "yeeloong_kb3310b",
+	.id = -1,
+	.dev = {
+		.platform_data = &yeeloong_ec_info,
+	},
+};
+
+int __init mach_platform_init(void)
+{
+	/*
+	 * arcs_cmdline is __initdata, which will be freed after boot and cannot
+	 * be used. We extract the EC version string from it, and pass it to
+	 * yeeloong-kb3310b driver as platform data.
+	 */
+	static const char token[] = "EC_VER=";
+	char *p;
+
+	p = strstr(arcs_cmdline, token);
+	if (!p)
+		memset(yeeloong_ec_info.version, 0, KB3310B_VERSION_LEN);
+	else {
+		p += ARRAY_SIZE(token) - 1;
+		strncpy(yeeloong_ec_info.version, p, KB3310B_VERSION_LEN);
+		p = strstr(yeeloong_ec_info.version, " ");
+		if (p)
+			*p = '\0';
+	}
+
+	return platform_device_register(&yeeloong_ec_device);
+}
-- 
2.20.1

[PATCH v2 6/7] mips: loongson64: Support System Control Interrupts for Lemote Yeeloong.

[Yifeng Li]

The hardware design of Yeeloong laptops is similar to OLPC: low-level
hardware events are processed by the ENE KB3310B Embedded Controller,
which is connected to the AMD CS5536 southbridge through a GPIO port.

When a hardware event occurs, such as a short-circuit on the USB port,
removing the power supply, plugging in a VGA adapter, opening the lid
of the laptop, or pressing a hotkey on the keyboard, the EC sends a
pulse to CS5536, which then fires a System Control Interrupt to notify
the kernel.

In the previous attempted submission, the logic for handling SCI and
hotkeys was tightly-coupled, and the driver was called the "hotkey"
driver, which was misleading. In this implementation of sci.c, we only
handle the minimum number of things that deal with the underlying
platform hardware directly, such as setting up the CS5536 GPIO and IRQ,
handling the the power supply of USB and camera. The vast majority of
events are passed to the subdrivers via a notification chain, thus,
the SCI logic and the specific hotkey handling logic have been decoupled.

Signed-off-by: Yifeng Li <tomli@tomli.me>
---
 arch/mips/loongson64/lemote-2f/Makefile |   2 +-
 arch/mips/loongson64/lemote-2f/sci.c    | 394 ++++++++++++++++++++++++
 2 files changed, 395 insertions(+), 1 deletion(-)
 create mode 100644 arch/mips/loongson64/lemote-2f/sci.c

diff --git a/arch/mips/loongson64/lemote-2f/Makefile b/arch/mips/loongson64/lemote-2f/Makefile
index 2b18752424ee..af4a1d347884 100644
--- a/arch/mips/loongson64/lemote-2f/Makefile
+++ b/arch/mips/loongson64/lemote-2f/Makefile
@@ -2,7 +2,7 @@
 # Makefile for lemote loongson2f family machines
 #
 
-obj-y += clock.o machtype.o irq.o reset.o dma.o platform.o
+obj-y += clock.o machtype.o irq.o reset.o dma.o platform.o sci.o
 
 #
 # Suspend Support
diff --git a/arch/mips/loongson64/lemote-2f/sci.c b/arch/mips/loongson64/lemote-2f/sci.c
new file mode 100644
index 000000000000..c78d579e72de
--- /dev/null
+++ b/arch/mips/loongson64/lemote-2f/sci.c
@@ -0,0 +1,394 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ * Support for Lemote Yeeloong System Control Interrupts (SCI)
+ *
+ * Copyright (C) 2009 Lemote Inc.
+ * Author: Wu Zhangjin <wuzhangjin@gmail.com>
+ *         Liu Junliang <liujl@lemote.com>
+ *
+ * Copyright (C) 2012, 2013, 2014, 2015 Petr Pisar
+ * Author: Petr Pisar <petr.pisar@atlas.cz> (bugfixes)
+ *
+ * Copyright (C) 2017 Jiaxun Yang
+ * Author: Jiaxun Yang <jiaxun.yang@flygoat.com>
+ *
+ * Copyright (C) 2019 Yifeng Li
+ * Author: Yifeng Li <tomli@tomli.me>
+ *
+ * The hardware design of Yeeloong laptops is similar to OLPC: low-level
+ * hardware events are processed by the ENE KB3310B Embedded Controller,
+ * which is connected to the AMD CS5536 southbridge through a GPIO port.
+ *
+ * When a hardware event occurs, such as a short-circuit on the USB port,
+ * removing the power supply, plugging in a VGA adapter, opening the lid
+ * of the laptop, or pressing a hotkey on the keyboard, the EC sends a
+ * pulse to CS5536, which then fires a System Control Interrupt to notify
+ * the kernel.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/export.h>
+#include <linux/power_supply.h>
+#include <linux/backlight.h>
+#include <linux/mfd/yeeloong_kb3310b.h>
+#include <linux/platform_device.h>
+#include <linux/reboot.h>
+#include <linux/suspend.h>
+
+#include <loongson.h>
+#include <cs5536/cs5536.h>
+
+static void usb_power_set(bool power)
+{
+	kb3310b_write(KB3310B_REG_USB0_FLAG, power);
+	kb3310b_write(KB3310B_REG_USB1_FLAG, power);
+	kb3310b_write(KB3310B_REG_USB2_FLAG, power);
+}
+
+static void camera_power_set(bool power)
+{
+	kb3310b_write(KB3310B_REG_CAMERA_CONTROL, power);
+}
+
+/*
+ * Handlers for EC events. We only handles the bare-minimum number of events
+ * directly related to the platform hardware. Other events are reported to
+ * subdrivers and handled by them.
+ */
+static void usb0_handler(void)
+{
+	pr_emerg("USB0 Overcurrent occurred!\n");
+}
+
+static void usb2_handler(void)
+{
+	pr_emerg("USB2 Overcurrent occurred!\n");
+}
+
+static void camera_handler(void)
+{
+	camera_power_set(!kb3310b_read(KB3310B_REG_CAMERA_CONTROL));
+}
+
+/*
+ * SCI notifiers. This notifier reports EC events to various
+ * subdrivers.
+ */
+static BLOCKING_NOTIFIER_HEAD(yeeloong_sci_notifier_list);
+
+int yeeloong_sci_register_notify(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&yeeloong_sci_notifier_list,
+						nb);
+}
+EXPORT_SYMBOL_GPL(yeeloong_sci_register_notify);
+
+int yeeloong_sci_unregister_notify(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&yeeloong_sci_notifier_list,
+						nb);
+}
+EXPORT_SYMBOL_GPL(yeeloong_sci_unregister_notify);
+
+/*
+ * Do not handle or notify other drivers about certain events while we're
+ * going down to reboot or sleep. This avoids unexpected behaviors in the
+ * userspace, such as closing the laptop lid while rebooting.
+ */
+static atomic_t reboot_flag;
+static atomic_t sleep_flag;
+
+static int
+notify_reboot(struct notifier_block *nb, unsigned long event, void *buf)
+{
+	switch (event) {
+	case SYS_RESTART:
+	case SYS_HALT:
+	case SYS_POWER_OFF:
+		atomic_set(&reboot_flag, 1);
+		break;
+	default:
+		return NOTIFY_DONE;
+	}
+
+	return NOTIFY_OK;
+}
+
+static int
+notify_pm(struct notifier_block *nb, unsigned long event, void *buf)
+{
+	switch (event) {
+	case PM_HIBERNATION_PREPARE:
+	case PM_SUSPEND_PREPARE:
+		atomic_inc(&sleep_flag);
+		break;
+	case PM_POST_HIBERNATION:
+	case PM_POST_SUSPEND:
+	case PM_RESTORE_PREPARE:        /* do we need this ?? */
+		atomic_dec(&sleep_flag);
+		break;
+	default:
+		return NOTIFY_DONE;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block reboot_notifier = {
+	.notifier_call = notify_reboot,
+};
+
+static struct notifier_block pm_notifier = {
+	.notifier_call = notify_pm,
+};
+
+static bool is_spurious_event(int event)
+{
+	if (event == KB3310B_EVENT_LID || event == KB3310B_EVENT_SLEEP)
+		return !!(atomic_read(&reboot_flag) | atomic_read(&sleep_flag));
+	else
+		return false;
+}
+
+/*
+ * Invoke the handler of the reported SCI event.
+ */
+static void process_sci_event(int event)
+{
+	if (is_spurious_event(event))
+		return;
+
+	switch (event) {
+	case KB3310B_EVENT_USB_OC0:
+		usb0_handler();
+		break;
+	case KB3310B_EVENT_USB_OC2:
+		usb2_handler();
+		break;
+	case KB3310B_EVENT_CAMERA:
+		camera_handler();
+		break;
+	default:
+		break;
+	}
+
+	/*
+	 * Report this event to other subdrivers, in particular, yeeloong-hotkey
+	 * driver will report the hotkey to userspace.
+	 */
+	blocking_notifier_call_chain(&yeeloong_sci_notifier_list, event, NULL);
+}
+
+/*
+ * Handle the SCI event and perform the needed action.
+ *
+ * A SCI event lasts about 120 microseconds. It means the function must take
+ * longer than 120 microseconds to complete. It has been shown that the function
+ * already takes 3 ms, so no artificial delay is needed.
+ */
+static irqreturn_t sci_irq_handler(int irq, void *dev_id)
+{
+	int ret, event;
+
+	if (irq != KB3310B_SCI_IRQ_NUM)
+		return IRQ_NONE;
+
+	/* query the event number */
+	ret = kb3310b_query_event_num();
+	if (ret < 0)
+		return IRQ_NONE;
+
+	event = kb3310b_get_event_num();
+	if (event < KB3310B_EVENT_START || event > KB3310B_EVENT_END)
+		return IRQ_NONE;
+
+	/* execute the corresponding action */
+	process_sci_event(event);
+
+	return IRQ_HANDLED;
+}
+
+/*
+ * Program the GPIO and MSR registers on CS5536.
+ *
+ * TODO: Linux kernel already has a cs5535-gpio kernel driver, but that
+ *       driver not adapted for Loongson. It's desirable to convert these
+ *       raw operations to use cs5535-gpio. Also, Loongson has its own
+ *       CS5536 clocksource driver, though Linux already has cs5535-clockevt,
+ *       but only supports clockevent, not clocksource. Clocksource code
+ *       should be merged into cs5535-clockevt and Loongson should use
+ *       that, too.
+ */
+static int setup_ec_sci(void)
+{
+	u32 hi, lo;
+	u32 gpio_base;
+	unsigned long flags;
+	int ret;
+
+	/* Get GPIO base */
+	_rdmsr(DIVIL_MSR_REG(DIVIL_LBAR_GPIO), &hi, &lo);
+	gpio_base = lo & 0xff00;
+
+	/*
+	 * HACK: to prevent any interrupts from being fired, we query
+	 * the EC to clear the pending event, and wait for a while to
+	 * miss the interrupt intentionally.
+	 */
+	ret = kb3310b_query_event_num();
+	if (ret)
+		return ret;
+
+	/* wait for a while */
+	mdelay(10);
+
+	/*
+	 * Set GPIO native registers and MSRs for GPIO-27 SCI EVENT PIN
+	 *
+	 * MSR:
+	 *     no primary and LPC.
+	 *     Unrestricted Z Input 8 to IG-10 from Virtual GPIO-0.
+	 *
+	 * GPIO mode:
+	 *     input, pull-up, no-invert, event-count and value 0,
+	 *     no-filter, no-edge. GPIO-27 map to Virtual GPIO-0.
+	 */
+	local_irq_save(flags);
+
+	/* set primary mask */
+	_rdmsr(0x80000024, &hi, &lo);
+	lo &= ~(1 << 10);
+	_wrmsr(0x80000024, hi, lo);
+
+	/* set LPC mask */
+	_rdmsr(0x80000025, &hi, &lo);
+	lo &= ~(1 << 10);
+	_wrmsr(0x80000025, hi, lo);
+
+	/* set Unrestricted Z map */
+	_rdmsr(0x80000023, &hi, &lo);
+	lo |= (0x0a << 0);
+	_wrmsr(0x80000023, hi, lo);
+
+	local_irq_restore(flags);
+
+	asm(".set noreorder\n");
+	outl(0x00000800, (gpio_base | 0xA0));  /* GPIO-27 input enable */
+	outl(0x00000800, (gpio_base | 0xA4));  /* GPIO-27 input invert */
+	outl(0x00000800, (gpio_base | 0xB8));  /* GPIO-27 event-int enable */
+	asm(".set reorder\n");
+
+	return 0;
+}
+
+/*
+ * Setup the IRQ handler for SCI. Must be called after setup_ec_sci().
+ */
+static int setup_sci_interrupt(struct platform_device *pdev)
+{
+	int ret;
+
+	ret = request_threaded_irq(KB3310B_SCI_IRQ_NUM, NULL, &sci_irq_handler,
+					IRQF_ONESHOT, "sci", NULL);
+	if (ret)
+		dev_err(&pdev->dev, "unable to request interrupt!\n");
+
+	return ret;
+}
+
+static int yeeloong_sci_probe(struct platform_device *pdev)
+{
+	int ret;
+
+	camera_power_set(KB3310B_BIT_CAMERA_CONTROL_OFF);
+	usb_power_set(KB3310B_BIT_USB_FLAG_ON);
+
+	ret = register_reboot_notifier(&reboot_notifier);
+	if (ret) {
+		dev_err(&pdev->dev, "unable to register reboot_notifier!\n");
+		goto fail;
+	}
+
+	ret = register_pm_notifier(&pm_notifier);
+	if (ret) {
+		dev_err(&pdev->dev, "unable to register pm_notifier!\n");
+		goto fail_reboot;
+	}
+
+	ret = setup_ec_sci();
+	if (ret) {
+		dev_err(&pdev->dev, "unable to setup EC SCI!\n");
+		goto fail_irq;
+	}
+
+	ret = setup_sci_interrupt(pdev);
+	if (ret)
+		goto fail_irq;
+
+	return ret;
+
+fail_irq:
+	free_irq(KB3310B_SCI_IRQ_NUM, NULL);
+	unregister_pm_notifier(&pm_notifier);
+fail_reboot:
+	unregister_reboot_notifier(&reboot_notifier);
+fail:
+	return ret;
+}
+
+#ifdef CONFIG_PM
+static int
+yeeloong_sci_suspend(struct device *dev)
+{
+	usb_power_set(KB3310B_BIT_USB_FLAG_OFF);
+	camera_power_set(KB3310B_BIT_CAMERA_CONTROL_OFF);
+	return 0;
+}
+
+static int yeeloong_sci_resume(struct device *dev)
+{
+	int ret;
+
+	usb_power_set(KB3310B_BIT_USB_FLAG_ON);
+
+	ret = setup_ec_sci();
+	if (ret) {
+		dev_err(dev, "unable to setup EC SCI!\n");
+		return -EFAULT;
+	}
+
+	/*
+	 * Lid switch and power supply may hawe changed while we were
+	 * asleep, so we generate a KB3310B_EVENT_LID and KB3310B_EVENT_AC_BAT
+	 * events to force the hotkey/battery subdriver to report their new
+	 * states.
+	 */
+	blocking_notifier_call_chain(&yeeloong_sci_notifier_list,
+						KB3310B_EVENT_LID, NULL);
+	blocking_notifier_call_chain(&yeeloong_sci_notifier_list,
+						KB3310B_EVENT_AC_BAT, NULL);
+
+	return 0;
+}
+
+static const SIMPLE_DEV_PM_OPS(yeeloong_sci_pm_ops,
+				yeeloong_sci_suspend, yeeloong_sci_resume);
+#endif
+
+static struct platform_driver yeeloong_sci_driver = {
+	.probe = yeeloong_sci_probe,
+	.driver = {
+		.name = "yeeloong_sci",
+#ifdef CONFIG_PM
+		.pm = &yeeloong_sci_pm_ops,
+#endif
+	},
+};
+
+static int __init yeeloong_sci_init(void)
+{
+	return platform_driver_register(&yeeloong_sci_driver);
+}
+arch_initcall(yeeloong_sci_init);
-- 
2.20.1

[PATCH v2 7/7] MAINTAINERS: add myself as a maintainer of MIPS/Loongson2 platform code.

[Yifeng Li]

I've introduced platform code for Lemote Yeeloong computers and modified
power management-related files. Add myself as a maintainer of these code.

Signed-off-by: Yifeng Li <tomli@tomli.me>
---
 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 208f19801a23..a82cd47927c1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10134,6 +10134,15 @@ F:	arch/mips/include/asm/mach-loongson64/
 F:	drivers/*/*loongson2*
 F:	drivers/*/*/*loongson2*
 
+MIPS/LOONGSON2 LEMOTE PLATFORM AND POWER MANAGEMENT DRIVER
+M:	Tom Li <tomli@tomli.me>
+L:	linux-mips@vger.kernel.org
+S:	Maintained
+F:	arch/mips/loongson64/common/platform.c
+F:	arch/mips/loongson64/lemote-2f/pm.c
+F:	arch/mips/loongson64/lemote-2f/sci.c
+F:	arch/mips/loongson64/lemote-2f/platform.c
+
 MIPS/LOONGSON3 ARCHITECTURE
 M:	Huacai Chen <chenhc@lemote.com>
 L:	linux-mips@vger.kernel.org
-- 
2.20.1

Re: [PATCH v2 1/7] mfd: yeeloong_kb3310b: support KB3310B EC for Lemote Yeeloong laptops.

[Paul Burton]

Hi Tom,

Overall I think this is much better than the older out of tree platform
driver - thanks for cleaning it up.

One general comment - it would be good to run the patches through
./scripts/checkpatch.pl & fix up any warnings it gives unless you have a
good reason to disagree with them.

On Tue, Mar 05, 2019 at 06:28:42AM +0800, Yifeng Li wrote:
> Lemote Yeeloong is a laptop powered by Loongson 2F MIPS processor,
> primarily a demo platform for hobbyists and developers. It uses an
> ENE KB3310B Embedded Controller with customized firmware to implement
> hardware and power management.
> 
> A monolithic platform driver code for those functionality has existed
> out-of-tree for many years. This commit creates a MFD driver for the EC
> chip on Yeeloong laptop to isolate EC-related code from core MIPS code,
> and serves as the foundation of various subdrivers.
> 
> My original attempt was to create a regmap for subdrivers to access the
> EC, unfortunately, the board files in Linux/MIPS still needs to access
> the EC directly for power management. Unless we find a better home for
> those code, we simply export the EC-related functions.

I'm not sure I follow why the power management code prevents use of
regmap?

Are you talking about the wakeup_loongson() function? Perhaps it would
make sense for the suspend code to be part of one of the possible
subdrivers you mention. The lemote-2f seems to be the only system that
provides an implementation of wakeup_loongson() so perhaps a driver
could instead just register its own struct platform_suspend_ops & avoid
the need for code in arch/mips to care about the EC.

> diff --git a/drivers/mfd/yeeloong_kb3310b.c b/drivers/mfd/yeeloong_kb3310b.c
> new file mode 100644
> index 000000000000..64d353a83122
> --- /dev/null
> +++ b/drivers/mfd/yeeloong_kb3310b.c
> @@ -0,0 +1,207 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +/*
> + * MFD driver for ENE KB3310B embedded controller on Lemote Yeeloong laptops
> + *
> + * Copyright (C) 2008 Lemote Inc.
> + * Author: liujl <liujl@lemote.com>, 2008-04-20
> + *
> + * Copyright (C) 2018 Yifeng Li
> + * Author: Yifeng Li <tomli@tomli.me>
> + *
> + * This is a MFD driver for the ENE KB3310B Embedded Controller for Lemote
> + * Yeeloong laptops to provide utility functions to access the chip from
> + * subdrivers, and handle events and interrupts in board files. This is a
> + * special-purpose driver, and it's only used on Lemote Yeeloong laptops,
> + * and is a mandatory dependency.
> + *
> + * My original attempt was to create a regmap for subdrivers to access the
> + * EC, unfortunately, the board files in Linux/MIPS still needs to access
> + * the EC directly for power management. Unless we find a better home for
> + * those code, we simply export the EC-related functions.
> + */
> +
> +#include <linux/export.h>
> +#include <linux/platform_device.h>
> +#include <linux/mfd/core.h>
> +#include <linux/io.h>
> +#include <linux/delay.h>
> +
> +#include <linux/mfd/yeeloong_kb3310b.h>
> +
> +#define DRV_NAME "yeeloong_kb3310b: "

Defining pr_fmt() would be cleaner - you wouldn't need to manually
include DRV_NAME in your messages later.

> +
> +/*****************************************************************************
> + * Most drivers, such as battery or backlight drivers, uses the I/O ports to
> + * access the Index Registers to obtain hardware status and information from
> + * EC chip.
> + ****************************************************************************/

Nit: the lines of asterisks aren't part of the kernel's general comment
style & I think it would looks cleaner to remove them.

> +static struct kb3310b_chip *kb3310b_fwinfo;
> +
> +static const struct mfd_cell kb3310b_cells[] = {
> +	{
> +		.name = "yeeloong_sci"
> +	},
> +	{
> +		.name = "yeeloong_hwmon"
> +	},
> +	{
> +		.name = "yeeloong_battery"
> +	},
> +	{
> +		.name = "yeeloong_backlight"
> +	},
> +	{
> +		.name = "yeeloong_lcd"
> +	},
> +	{
> +		.name = "yeeloong_hotkey"
> +	},

Nit: I think it'd look cleaner if you remove the newlines within each
array entry, eg:

	{ .name = "yeeloong_sci" },
	{ .name = "yeelong_hwmon" },
	...

> +};
> +
> +static DEFINE_SPINLOCK(kb3310b_index_lock);
> +
> +u8 kb3310b_read(u16 reg)
> +{
> +	unsigned long flags;
> +	u8 val;
> +
> +	spin_lock_irqsave(&kb3310b_index_lock, flags);
> +
> +	outb((reg & 0xff00) >> 8, KB3310B_IO_PORT_HIGH);
> +	outb((reg & 0x00ff), KB3310B_IO_PORT_LOW);
> +	val = inb(KB3310B_IO_PORT_DATA);
> +
> +	spin_unlock_irqrestore(&kb3310b_index_lock, flags);
> +
> +	return val;
> +}
> +EXPORT_SYMBOL_GPL(kb3310b_read);
> +
> +void kb3310b_write(u16 reg, u8 val)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&kb3310b_index_lock, flags);
> +
> +	outb((reg & 0xff00) >> 8, KB3310B_IO_PORT_HIGH);
> +	outb((reg & 0x00ff), KB3310B_IO_PORT_LOW);
> +	outb(val, KB3310B_IO_PORT_DATA);
> +	inb(KB3310B_IO_PORT_DATA);  /* flush pending writes */
> +
> +	spin_unlock_irqrestore(&kb3310b_index_lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(kb3310b_write);

If you do switch to using regmap then the above could be neatly
represented as a bus - see struct regmap_bus, and various users of it
throughout drivers/. regmap would even handle the locking for you.

> +
> +bool kb3310b_fw_earlier(char *version)
> +{
> +	return (strncasecmp(kb3310b_fwinfo->version,
> +				version, KB3310B_VERSION_LEN) < 0);
> +}
> +EXPORT_SYMBOL_GPL(kb3310b_fw_earlier);
> +
> +static int kb3310b_probe(struct platform_device *pdev)
> +{
> +	kb3310b_fwinfo = dev_get_platdata(&pdev->dev);
> +	pr_info(DRV_NAME "firmware version %s", kb3310b_fwinfo->version);
> +
> +	return devm_mfd_add_devices(&pdev->dev, -1, kb3310b_cells,
> +				    ARRAY_SIZE(kb3310b_cells), NULL, 0, NULL);
> +}
> +
> +static struct platform_driver kb3310b_driver = {
> +	.driver = {
> +		   .name = "yeeloong_kb3310b",
> +	},
> +	.probe = kb3310b_probe,
> +};
> +builtin_platform_driver(kb3310b_driver);
> +
> +/*****************************************************************************
> + * For interrupt handling and power management, the EC chip is also needed to
> + * be queried from the board file at arch/mips/loongson64, through a separate
> + * command port.
> + *****************************************************************************/

As before, I think it'd be cleaner without the lines of asterisks.
Probably without the empty line before the variable too.

> +
> +static DEFINE_SPINLOCK(kb3310b_command_lock);

Since this is only used in kb3310b_query_seq() could you just declare it
(still static) inside that function?

Thanks,
    Paul

> +
> +/*
> + * This function is used for EC command writes and corresponding status queries.
> + */
> +int kb3310b_query_seq(unsigned char cmd)
> +{
> +	int timeout;
> +	unsigned char status;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&kb3310b_command_lock, flags);
> +
> +	/* make chip goto reset mode */
> +	udelay(KB3310B_REG_UDELAY);
> +	outb(cmd, KB3310B_CMD_PORT);
> +	udelay(KB3310B_REG_UDELAY);
> +
> +	/* check if the command is received by EC */
> +	timeout = KB3310B_CMD_TIMEOUT;
> +	status = inb(KB3310B_STS_PORT);
> +	while (timeout-- && (status & (1 << 1))) {
> +		status = inb(KB3310B_STS_PORT);
> +		udelay(KB3310B_REG_UDELAY);
> +	}
> +
> +	spin_unlock_irqrestore(&kb3310b_command_lock, flags);
> +
> +	if (timeout <= 0) {
> +		pr_err(DRV_NAME
> +			"(%x/NA) failed to issue command %d, no response!\n",
> +			timeout, cmd);
> +		return -EINVAL;
> +	}
> +
> +	pr_info(DRV_NAME
> +		 "(%x/%x) issued command %d, status: 0x%x\n",
> +		 timeout, KB3310B_CMD_TIMEOUT - timeout,
> +		 cmd, status);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(kb3310b_query_seq);
> +
> +/*
> + * Send query command to EC to get the proper event number.
> + */
> +int kb3310b_query_event_num(void)
> +{
> +	return kb3310b_query_seq(KB3310B_CMD_GET_EVENT_NUM);
> +}
> +EXPORT_SYMBOL_GPL(kb3310b_query_event_num);
> +
> +/*
> + * Get event number from EC.
> + *
> + * NOTE: This routine must follow the query_event_num function in the
> + * interrupt.
> + */
> +int kb3310b_get_event_num(void)
> +{
> +	int timeout = 100;
> +	unsigned char value;
> +	unsigned char status;
> +
> +	udelay(KB3310B_REG_UDELAY);
> +	status = inb(KB3310B_STS_PORT);
> +	udelay(KB3310B_REG_UDELAY);
> +	while (timeout-- && !(status & (1 << 0))) {
> +		status = inb(KB3310B_STS_PORT);
> +		udelay(KB3310B_REG_UDELAY);
> +	}
> +	if (timeout <= 0) {
> +		pr_info("%s: get event number timeout.\n", __func__);
> +		return -EINVAL;
> +	}
> +	value = inb(KB3310B_DAT_PORT);
> +	udelay(KB3310B_REG_UDELAY);
> +
> +	return value;
> +}
> +EXPORT_SYMBOL_GPL(kb3310b_get_event_num);
> diff --git a/include/linux/mfd/yeeloong_kb3310b.h b/include/linux/mfd/yeeloong_kb3310b.h
> new file mode 100644
> index 000000000000..1f16ba2579bc
> --- /dev/null
> +++ b/include/linux/mfd/yeeloong_kb3310b.h
> @@ -0,0 +1,211 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +/*
> + * MFD driver for ENE KB3310B embedded controller on Lemote Yeeloong laptops
> + *
> + * Copyright (C) 2008 Lemote Inc.
> + * Author: liujl <liujl@lemote.com>, 2008-04-20
> + *
> + * Copyright (C) 2018 Yifeng Li
> + * Author: Yifeng Li <tomli@tomli.me>
> + */
> +
> +#ifndef __LINUX_MFD_YEELOONG_KB3310B_H
> +#define __LINUX_MFD_YEELOONG_KB3310B_H
> +
> +extern u8 kb3310b_read(u16 reg);
> +extern void kb3310b_write(u16 reg, u8 val);
> +extern bool kb3310b_fw_earlier(char *version);
> +extern int kb3310b_query_seq(unsigned char cmd);
> +extern int kb3310b_query_event_num(void);
> +extern int kb3310b_get_event_num(void);
> +
> +typedef int (*sci_handler) (int status);
> +
> +extern int yeeloong_sci_register_notify(struct notifier_block *nb);
> +extern int yeeloong_sci_unregister_notify(struct notifier_block *nb);
> +
> +#define KB3310B_VERSION_LEN 8
> +
> +struct kb3310b_chip {
> +	char version[KB3310B_VERSION_LEN];
> +};
> +
> +#define KB3310B_SCI_IRQ_NUM	0x0A
> +
> +/*
> + * The following registers are determined by the EC index configuration.
> + * 1, fill the PORT_HIGH as EC register high part.
> + * 2, fill the PORT_LOW as EC register low part.
> + * 3, fill the PORT_DATA as EC register write data or get the data from it.
> + */
> +#define KB3310B_IO_PORT_HIGH	0x0381
> +#define KB3310B_IO_PORT_LOW	0x0382
> +#define KB3310B_IO_PORT_DATA	0x0383
> +
> +/*
> + * EC delay time is 500us for register and status access
> + */
> +#define KB3310B_REG_UDELAY	500
> +#define KB3310B_CMD_TIMEOUT	0x1000
> +
> +/*
> + * EC access port for SCI communication
> + */
> +#define KB3310B_CMD_PORT		0x66
> +#define KB3310B_STS_PORT		0x66
> +#define KB3310B_DAT_PORT		0x62
> +#define KB3310B_CMD_INIT_IDLE_MODE	0xdd
> +#define KB3310B_CMD_EXIT_IDLE_MODE	0xdf
> +#define KB3310B_CMD_INIT_RESET_MODE	0xd8
> +#define KB3310B_CMD_REBOOT_SYSTEM	0x8c
> +#define KB3310B_CMD_GET_EVENT_NUM	0x84
> +#define KB3310B_CMD_PROGRAM_PIECE	0xda
> +
> +/* temperature & fan registers */
> +#define KB3310B_REG_TEMPERATURE_VALUE	0xF458
> +#define KB3310B_REG_FAN_AUTO_MAN_SWITCH 0xF459
> +#define KB3310B_BIT_FAN_AUTO		0
> +#define KB3310B_BIT_FAN_MANUAL		1
> +#define KB3310B_REG_FAN_CONTROL		0xF4D2
> +#define KB3310B_BIT_FAN_CONTROL_ON	(1 << 0)
> +#define KB3310B_BIT_FAN_CONTROL_OFF	(0 << 0)
> +#define KB3310B_REG_FAN_STATUS		0xF4DA
> +#define KB3310B_BIT_FAN_STATUS_ON	(1 << 0)
> +#define KB3310B_BIT_FAN_STATUS_OFF	(0 << 0)
> +#define KB3310B_REG_FAN_SPEED_HIGH	0xFE22
> +#define KB3310B_REG_FAN_SPEED_LOW	0xFE23
> +#define KB3310B_REG_FAN_SPEED_LEVEL	0xF4CC
> +
> +/* fan speed divider */
> +#define KB3310B_FAN_SPEED_DIVIDER	480000	/* (60*1000*1000/62.5/2)*/
> +
> +/* battery registers */
> +#define KB3310B_REG_BAT_DESIGN_CAP_HIGH		0xF77D
> +#define KB3310B_REG_BAT_DESIGN_CAP_LOW		0xF77E
> +#define KB3310B_REG_BAT_FULLCHG_CAP_HIGH	0xF780
> +#define KB3310B_REG_BAT_FULLCHG_CAP_LOW		0xF781
> +#define KB3310B_REG_BAT_DESIGN_VOL_HIGH		0xF782
> +#define KB3310B_REG_BAT_DESIGN_VOL_LOW		0xF783
> +#define KB3310B_REG_BAT_CURRENT_HIGH		0xF784
> +#define KB3310B_REG_BAT_CURRENT_LOW		0xF785
> +#define KB3310B_REG_BAT_VOLTAGE_HIGH		0xF786
> +#define KB3310B_REG_BAT_VOLTAGE_LOW		0xF787
> +#define KB3310B_REG_BAT_TEMPERATURE_HIGH	0xF788
> +#define KB3310B_REG_BAT_TEMPERATURE_LOW		0xF789
> +#define KB3310B_REG_BAT_RELATIVE_CAP_HIGH	0xF492
> +#define KB3310B_REG_BAT_RELATIVE_CAP_LOW	0xF493
> +#define KB3310B_REG_BAT_VENDOR			0xF4C4
> +#define KB3310B_FLAG_BAT_VENDOR_SANYO		0x01
> +#define KB3310B_FLAG_BAT_VENDOR_SIMPLO		0x02
> +#define KB3310B_REG_BAT_CELL_COUNT		0xF4C6
> +#define KB3310B_FLAG_BAT_CELL_3S1P		0x03
> +#define KB3310B_FLAG_BAT_CELL_3S2P		0x06
> +#define KB3310B_REG_BAT_CHARGE			0xF4A2
> +#define KB3310B_FLAG_BAT_CHARGE_DISCHARGE	0x01
> +#define KB3310B_FLAG_BAT_CHARGE_CHARGE		0x02
> +#define KB3310B_FLAG_BAT_CHARGE_ACPOWER		0x00
> +#define KB3310B_REG_BAT_STATUS			0xF4B0
> +#define KB3310B_BIT_BAT_STATUS_LOW		(1 << 5)
> +#define KB3310B_BIT_BAT_STATUS_DESTROY		(1 << 2)
> +#define KB3310B_BIT_BAT_STATUS_FULL		(1 << 1)
> +#define KB3310B_BIT_BAT_STATUS_IN		(1 << 0)
> +#define KB3310B_REG_BAT_CHARGE_STATUS		0xF4B1
> +#define KB3310B_BIT_BAT_CHARGE_STATUS_OVERTEMP	(1 << 2)
> +#define KB3310B_BIT_BAT_CHARGE_STATUS_PRECHG	(1 << 1)
> +#define KB3310B_REG_BAT_STATE			0xF482
> +#define KB3310B_BIT_BAT_STATE_CHARGING		(1 << 1)
> +#define KB3310B_BIT_BAT_STATE_DISCHARGING	(1 << 0)
> +#define KB3310B_REG_BAT_POWER			0xF440
> +#define KB3310B_BIT_BAT_POWER_S3		(1 << 2)
> +#define KB3310B_BIT_BAT_POWER_ON		(1 << 1)
> +#define KB3310B_BIT_BAT_POWER_ACIN		(1 << 0)
> +
> +/* other registers */
> +
> +/* Audio: rd/wr */
> +#define KB3310B_REG_AUDIO_VOLUME	0xF46C
> +#define KB3310B_REG_AUDIO_MUTE		0xF4E7
> +#define KB3310B_REG_AUDIO_BEEP		0xF4D0
> +
> +/* USB port power or not: rd/wr */
> +#define KB3310B_REG_USB0_FLAG		0xF461
> +#define KB3310B_REG_USB1_FLAG		0xF462
> +#define KB3310B_REG_USB2_FLAG		0xF463
> +#define KB3310B_BIT_USB_FLAG_ON		1
> +#define KB3310B_BIT_USB_FLAG_OFF	0
> +
> +/* LID */
> +#define KB3310B_REG_LID_DETECT		0xF4BD
> +#define KB3310B_BIT_LID_DETECT_ON	1
> +#define KB3310B_BIT_LID_DETECT_OFF	0
> +
> +/* CRT */
> +#define KB3310B_REG_CRT_DETECT		0xF4AD
> +#define KB3310B_BIT_CRT_DETECT_PLUG	1
> +#define KB3310B_BIT_CRT_DETECT_UNPLUG	0
> +
> +/* LCD backlight brightness adjust: 9 levels */
> +#define KB3310B_REG_DISPLAY_BRIGHTNESS	0xF4F5
> +
> +/* Black screen status */
> +#define KB3310B_REG_DISPLAY_LCD		0xF79F
> +#define KB3310B_BIT_DISPLAY_LCD_ON	1
> +#define KB3310B_BIT_DISPLAY_LCD_OFF	0
> +
> +/* LCD backlight control: off/restore */
> +#define KB3310B_REG_BACKLIGHT_CTRL	0xF7BD
> +#define KB3310B_BIT_BACKLIGHT_ON	1
> +#define KB3310B_BIT_BACKLIGHT_OFF	0
> +
> +/* Reset the machine auto-clear: rd/wr */
> +#define KB3310B_REG_RESET		0xF4EC
> +#define KB3310B_BIT_RESET_ON		1
> +
> +/* Light the LED: rd/wr */
> +#define KB3310B_REG_LED			0xF4C8
> +#define KB3310B_BIT_LED_RED_POWER	(1 << 0)
> +#define KB3310B_BIT_LED_ORANGE_POWER	(1 << 1)
> +#define KB3310B_BIT_LED_GREEN_CHARGE	(1 << 2)
> +#define KB3310B_BIT_LED_RED_CHARGE	(1 << 3)
> +#define KB3310B_BIT_LED_NUMLOCK		(1 << 4)
> +
> +/* Test LED mode, all LED on/off */
> +#define KB3310B_REG_LED_TEST		0xF4C2
> +#define KB3310B_BIT_LED_TEST_IN		1
> +#define KB3310B_BIT_LED_TEST_OUT	0
> +
> +/* Camera on/off */
> +#define KB3310B_REG_CAMERA_STATUS	0xF46A
> +#define KB3310B_BIT_CAMERA_STATUS_ON	1
> +#define KB3310B_BIT_CAMERA_STATUS_OFF	0
> +#define KB3310B_REG_CAMERA_CONTROL	0xF7B7
> +#define KB3310B_BIT_CAMERA_CONTROL_OFF	0
> +#define KB3310B_BIT_CAMERA_CONTROL_ON	1
> +
> +/* WLAN Status */
> +#define KB3310B_REG_WLAN		0xF4FA
> +#define KB3310B_BIT_WLAN_ON		1
> +#define KB3310B_BIT_WLAN_OFF		0
> +
> +/* SCI Event Number from EC */
> +enum {
> +	KB3310B_EVENT_START = 0x22,
> +	KB3310B_EVENT_LID = 0x23,	   /* LID open/close */
> +	KB3310B_EVENT_DISPLAY_TOGGLE,	   /* Fn+F3 for display switch */
> +	KB3310B_EVENT_SLEEP,		   /* Fn+F1 for entering sleep mode */
> +	KB3310B_EVENT_OVERTEMP,		   /* Over-temperature occurred */
> +	KB3310B_EVENT_CRT_DETECT,	   /* CRT is connected */
> +	KB3310B_EVENT_CAMERA,		   /* Camera on/off */
> +	KB3310B_EVENT_USB_OC2,		   /* USB2 Overcurrent occurred */
> +	KB3310B_EVENT_USB_OC0,		   /* USB0 Overcurrent occurred */
> +	KB3310B_EVENT_BLACK_SCREEN,	   /* Turn on/off backlight */
> +	KB3310B_EVENT_AUDIO_MUTE,	   /* Mute on/off */
> +	KB3310B_EVENT_DISPLAY_BRIGHTNESS,  /* LCD backlight brightness adjust */
> +	KB3310B_EVENT_AC_BAT,		   /* AC & Battery relative issue */
> +	KB3310B_EVENT_AUDIO_VOLUME,	   /* Volume adjust */
> +	KB3310B_EVENT_WLAN,		   /* WLAN on/off */
> +	KB3310B_EVENT_END
> +};
> +
> +#endif /* !__LINUX_MFD_YEELOONG_KB3310B_H */
> -- 
> 2.20.1
> 

Re: [PATCH v2 1/7] mfd: yeeloong_kb3310b: support KB3310B EC for Lemote Yeeloong laptops.

[Tom Li]

On Tue, Mar 05, 2019 at 11:50:51PM +0000, Paul Burton wrote:
> Hi Tom,
> 
> Overall I think this is much better than the older out of tree platform
> driver - thanks for cleaning it up.

I'm glad to hear that.

> One general comment - it would be good to run the patches through
> ./scripts/checkpatch.pl & fix up any warnings it gives unless you have a
> good reason to disagree with them.

The entire patchset has already been checked by checkpatch.pl, and all
generated warnings have been corrected before submission. So I don't think
the script can help me improving the style beyond this point. Thanks for
pointing out all the additional style issues.

> Nit: the lines of asterisks aren't part of the kernel's general comment
> style & I think it would looks cleaner to remove them.

Sure, I will remove these asterisks banners.

In fact I learned to use these asterisks from other kernel drivers to
separate logical sections... and as a said, checkpatch.pl does not generate
a warning about it, so I suspect that, historically, a clear policy about
asterisks banners was never enforced. After finish working on these patches,
perhaps it would a good time to patch checkpatch.pl and add a check for it?

> I'm not sure I follow why the power management code prevents use of
> regmap?
> 
> Are you talking about the wakeup_loongson() function? Perhaps it would
> make sense for the suspend code to be part of one of the possible
> subdrivers you mention. The lemote-2f seems to be the only system that
> provides an implementation of wakeup_loongson() so perhaps a driver
> could instead just register its own struct platform_suspend_ops & avoid
> the need for code in arch/mips to care about the EC.

I'll reword my misleading commit message about using regmap.

Let me clarify it - nothing prevents the use of regmap. I originally thought
it would be a good idea since the manual locking can be eliminated.

But later I realized it would mean more changes. In particular, not only pm.c
needs to be a new subdriver, the shutdown code in ml2f_reboot() also calls
ec_write(), and it needs to be converted to a subdriver in drivers/power/reset/
too. Yes, it would be good to refactor these code and move them to a more
suitable place, but currently, the primary goal is to add platform drivers,
not to touch functions I've just fixed and possibly introduce new regressions...

As you've seen, there is a TODO item in [PATCH 6/7] in this series, it says the
CS5536 code also needs some major refactoring: the GPIO needs to be changed,
the clocksource driver needs to be merged with the clockevent driver under
drivers/clocksource, etc. I think we can, as well, save the pm.c/reset.c for
later. Currently, I think it's better to focus on the platform drivers first.

If you still have objections on it, please let me know.

> > +#define DRV_NAME "yeeloong_kb3310b: "
> 
> Defining pr_fmt() would be cleaner - you wouldn't need to manually
> include DRV_NAME in your messages later.

Noted.

> > +static struct kb3310b_chip *kb3310b_fwinfo;
> > +
> > +static const struct mfd_cell kb3310b_cells[] = {
> > +	{
> > +		.name = "yeeloong_sci"
> > +	},
> > +	{
> > +		.name = "yeeloong_hwmon"
> > +	},
> > +	{
> > +		.name = "yeeloong_battery"
> > +	},
> > +	{
> > +		.name = "yeeloong_backlight"
> > +	},
> > +	{
> > +		.name = "yeeloong_lcd"
> > +	},
> > +	{
> > +		.name = "yeeloong_hotkey"
> > +	},
> 
> Nit: I think it'd look cleaner if you remove the newlines within each
> array entry, eg:
> 
> 	{ .name = "yeeloong_sci" },
> 	{ .name = "yeelong_hwmon" },
> 	...

Yes.

> > +
> > +static DEFINE_SPINLOCK(kb3310b_command_lock);
> 
> Since this is only used in kb3310b_query_seq() could you just declare it
> (still static) inside that function?

No problem.

Thanks for your review, I'll correct all the issues above and send v3 today,
if there's no addition problem in v3, I will start sending the actual platform
drivers (battery/hwmon/etc) for the next round of review.

P.S: This time, I hope Lee Jones, as the MFD subsystem maintainer, has
received my mail and my patch, including this one. Unfortunately, all signs
indicated he hasn't received it. Jones, if you have received this mail but
currently too busy to review, please reply to confirm, thanks!

Sincerely yours,

Tom Li