[PATCH v6 0/2] ZII RAVE platform driver

[PATCH v6 0/2] ZII RAVE platform driver

[Andrey Smirnov]

Hi everyone,

This patch series is v6 of the driver for supervisory processor found
on RAVE series of devices from ZII. Supervisory processor is a PIC
microcontroller connected to various electrical subsystems on RAVE
devices whose firmware implements protocol to command/qery them.

Changes since [v5]:

    - Fixed a build break, introduced by a last minute change in [v5]

    - Moved majority of attributes that were exposed over sysfs to debugfs

    - Document remaining sysfs attributes in Documentation/ABI/testing/sysfs-platform-rave-sp

Changes since [v4]:

    - Replaced usage of DEVICE_ATTR with DEVICE_ATTR_RW

    - Fixed a number of warnings produces by sparse tool

    - Incorporated event more feedback from Andy Shevchenko

    - Collected Reviewed-by from Andy

Changes since [v3]:

    - Re-collected lost Acked-by from Rob

    - Incorporated further feedback from Andy Shevchenko

    - Dropped useless change (stray newline) to drivers/mfd/Makefile

Changes since [v2]:

    - Fixed swapped command codes in rave_sp_common_get_boot_source()
      and rave_sp_common_set_boot_source() revealed by further testing
      of the code

    - Incorporated feedback from Andy Shevchenko

Changes since [v1]:

    - Updated wording in DT-bindings as per Rob's request.

    - Collected Rob's Acked-by for patch 2/2

NOTE:

 * The driver for "zii,rave-sp-watchdog" exists, but I haven't
   submitted it yet, becuase I wanted to make sure that API exposed by
   this MFD is acceptable and doesn't need drastic changes.

 * This driver is dependent on crc_ccitt_false() introduced in
   2da9378d531f8cc6670c7497f20d936b706ab80b in 'linux-next'

Feedback is greatly appreciated!

Thanks,
Andrey Smirnov

[v5] lkml.kernel.org/r/20170728142704.11156-1-andrew.smirnov@gmail.com
[v4] lkml.kernel.org/r/20170725184450.13171-1-andrew.smirnov@gmail.com
[v3] lkml.kernel.org/r/20170724150915.4824-1-andrew.smirnov@gmail.com
[v2] lkml.kernel.org/r/20170718175604.11735-1-andrew.smirnov@gmail.com
[v1] lkml.kernel.org/r/20170710170449.4544-1-andrew.smirnov@gmail.com

Andrey Smirnov (2):
  platform: Add driver for RAVE Supervisory Processor
  dt-bindings: mfd: Add bindings for ZII RAVE devices

 Documentation/ABI/testing/sysfs-platform-rave-sp   |   35 +
 .../devicetree/bindings/mfd/zii,rave-sp.txt        |   39 +
 drivers/platform/Kconfig                           |    2 +
 drivers/platform/Makefile                          |    1 +
 drivers/platform/rave/Kconfig                      |   26 +
 drivers/platform/rave/Makefile                     |    1 +
 drivers/platform/rave/rave-sp.c                    | 1190 ++++++++++++++++++++
 include/linux/rave-sp.h                            |   54 +
 8 files changed, 1348 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-platform-rave-sp
 create mode 100644 Documentation/devicetree/bindings/mfd/zii,rave-sp.txt
 create mode 100644 drivers/platform/rave/Kconfig
 create mode 100644 drivers/platform/rave/Makefile
 create mode 100644 drivers/platform/rave/rave-sp.c
 create mode 100644 include/linux/rave-sp.h

-- 
2.13.5

[PATCH v6 1/2] platform: Add driver for RAVE Supervisory Processor

[Andrey Smirnov]

Add a driver for RAVE Supervisory Processor, an MCU implementing
varoius bits of housekeeping functionality (watchdoging, backlight
control, LED control, etc) on RAVE family of products by Zodiac
Inflight Innovations.

This driver implementes core MFD/serdev device as well as
communication subroutines necessary for commanding the device.

Cc: linux-kernel@vger.kernel.org
Cc: cphealy@gmail.com
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Pavel Machek <pavel@ucw.cz>
Tested-by: Chris Healy <cphealy@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 Documentation/ABI/testing/sysfs-platform-rave-sp |   35 +
 drivers/platform/Kconfig                         |    2 +
 drivers/platform/Makefile                        |    1 +
 drivers/platform/rave/Kconfig                    |   26 +
 drivers/platform/rave/Makefile                   |    1 +
 drivers/platform/rave/rave-sp.c                  | 1190 ++++++++++++++++++++++
 include/linux/rave-sp.h                          |   54 +
 7 files changed, 1309 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-platform-rave-sp
 create mode 100644 drivers/platform/rave/Kconfig
 create mode 100644 drivers/platform/rave/Makefile
 create mode 100644 drivers/platform/rave/rave-sp.c
 create mode 100644 include/linux/rave-sp.h

diff --git a/Documentation/ABI/testing/sysfs-platform-rave-sp b/Documentation/ABI/testing/sysfs-platform-rave-sp
new file mode 100644
index 000000000000..81bdc54ba857
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-rave-sp
@@ -0,0 +1,35 @@
+What:		/sys/devices/platform/*/serial*/serial*-*/reset_reason
+Date:		Aug 2017
+Contact:	"Andrey Smirnov" <andrew.smirnov@gmail.com>
+Description:    (RO) Indicates reason for last reset.
+
+		The following values can be reported:
+		    * 0  -> Normal Power Off
+		    * 1  -> Hardware Watchdog
+		    * 2  -> Software Watchdog
+		    * 3  -> Input Voltage Out Of Range
+		    * 4  -> Host Requested
+		    * 5  -> Temperature Out Of Range
+		    * 6  -> User Requested (via long Power Button press)
+		    * 7  -> Illegal Configuration Word
+		    * 8  -> Illegal Insturction
+		    * 9  -> Illegal Trap
+		    * 10 -> Unknown
+		    * 11 -> Crew Panel Requested
+
+		 NOTE: Codes 7,8 and 9 pertain to exceptions happening
+		 in supervisory processor/its firmware and not host
+		 CPU.
+
+What:		/sys/devices/platform/*/serial*/serial*-*/boot_source
+Date:		Aug 2017
+Contact:	"Andrey Smirnov" <andrew.smirnov@gmail.com>
+Description:    (RW) Indicates currently selected boot source.
+
+		The following values are valid:
+		    * 0  -> SD card
+		    * 1  -> eMMC
+		    * 2  -> SPI NOR
+
+		NOTE: Setting boot source on RDU1 hardware is
+		currently not implemented
diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
index c11db8bceea1..e6db685bb895 100644
--- a/drivers/platform/Kconfig
+++ b/drivers/platform/Kconfig
@@ -8,3 +8,5 @@ endif
 source "drivers/platform/goldfish/Kconfig"
 
 source "drivers/platform/chrome/Kconfig"
+
+source "drivers/platform/rave/Kconfig"
diff --git a/drivers/platform/Makefile b/drivers/platform/Makefile
index ca2692510733..17bdec5ece0c 100644
--- a/drivers/platform/Makefile
+++ b/drivers/platform/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_MIPS)		+= mips/
 obj-$(CONFIG_OLPC)		+= olpc/
 obj-$(CONFIG_GOLDFISH)		+= goldfish/
 obj-$(CONFIG_CHROME_PLATFORMS)	+= chrome/
+obj-y += rave/
diff --git a/drivers/platform/rave/Kconfig b/drivers/platform/rave/Kconfig
new file mode 100644
index 000000000000..6fc50ade3871
--- /dev/null
+++ b/drivers/platform/rave/Kconfig
@@ -0,0 +1,26 @@
+#
+# Platform support for Zodiac RAVE hardware
+#
+
+menuconfig RAVE_PLATFORMS
+	bool "Platform support for Zodiac RAVE hardware"
+	depends on SERIAL_DEV_BUS && SERIAL_DEV_CTRL_TTYPORT
+	help
+	  Say Y here to get to see options for platform support for
+	  various devices present in RAVE hardware. This option alone
+	  does not add any kernel code.
+
+	  If you say N, all options in this submenu will be skipped
+	  and disabled.
+
+if RAVE_PLATFORMS
+
+config RAVE_SP_CORE
+	tristate "RAVE SP MCU core driver"
+	select MFD_CORE
+	select CRC_CCITT
+	help
+	  Select this to get support for the Supervisory Processor
+	  device found on several devices in RAVE line of hardware.
+
+endif
diff --git a/drivers/platform/rave/Makefile b/drivers/platform/rave/Makefile
new file mode 100644
index 000000000000..e4c21ab2d2f5
--- /dev/null
+++ b/drivers/platform/rave/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o
diff --git a/drivers/platform/rave/rave-sp.c b/drivers/platform/rave/rave-sp.c
new file mode 100644
index 000000000000..14d4e5cf2e5e
--- /dev/null
+++ b/drivers/platform/rave/rave-sp.c
@@ -0,0 +1,1190 @@
+/*
+ * Multifunction core driver for Zodiac Inflight Innovations
+ * SP MCU that is connected via dedicated UART port
+ *
+ * Copyright (C) 2017 Zodiac Inflight Innovations
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/atomic.h>
+#include <linux/crc-ccitt.h>
+#include <linux/delay.h>
+#include <linux/debugfs.h>
+#include <linux/export.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/rave-sp.h>
+#include <linux/sched.h>
+#include <linux/serdev.h>
+#include <asm/unaligned.h>
+
+/*
+ * UART protocol using following entities:
+ *  - message to MCU => ACK response
+ *  - event from MCU => event ACK
+ *
+ * Frame structure:
+ * <STX> <DATA> <CHECKSUM> <ETX>
+ * Where:
+ * - STX - is start of transmission character
+ * - ETX - end of transmission
+ * - DATA - payload
+ * - CHECKSUM - checksum calculated on <DATA>
+ *
+ * If <DATA> or <CHECKSUM> contain one of control characters, then it is
+ * escaped using <DLE> control code. Added <DLE> does not participate in
+ * checksum calculation.
+ */
+#define RAVE_SP_STX			0x02
+#define RAVE_SP_ETX			0x03
+#define RAVE_SP_DLE			0x10
+
+#define RAVE_SP_MAX_DATA_SIZE		64
+#define RAVE_SP_CHECKSUM_SIZE		2  /* Worst case scenario on RDU2 */
+/*
+ * We don't store STX, ETX and unescaped bytes, so Rx is only
+ * DATA + CSUM
+ */
+#define RAVE_SP_RX_BUFFER_SIZE				\
+	(RAVE_SP_MAX_DATA_SIZE + RAVE_SP_CHECKSUM_SIZE)
+
+#define RAVE_SP_STX_ETX_SIZE		2
+/*
+ * For Tx we have to have space for everything, STX, EXT and
+ * potentially stuffed DATA + CSUM data + csum
+ */
+#define RAVE_SP_TX_BUFFER_SIZE				\
+	(RAVE_SP_STX_ETX_SIZE + 2 * RAVE_SP_RX_BUFFER_SIZE)
+
+#define RAVE_SP_BOOT_SOURCE_GET		0
+#define RAVE_SP_BOOT_SOURCE_SET		1
+
+#define RAVE_SP_RDU2_BOARD_TYPE_RMB	0
+#define RAVE_SP_RDU2_BOARD_TYPE_DEB	1
+
+#define RAVE_SP_BOOT_SOURCE_SD		0
+#define RAVE_SP_BOOT_SOURCE_EMMC	1
+#define RAVE_SP_BOOT_SOURCE_NOR		2
+
+/**
+ * enum rave_sp_deframer_state - Possible state for de-framer
+ *
+ * @RAVE_SP_EXPECT_SOF:		 Scanning input for start-of-frame marker
+ * @RAVE_SP_EXPECT_DATA:	 Got start of frame marker, collecting frame
+ * @RAVE_SP_EXPECT_ESCAPED_DATA: Got escape character, collecting escaped byte
+ */
+enum rave_sp_deframer_state {
+	RAVE_SP_EXPECT_SOF,
+	RAVE_SP_EXPECT_DATA,
+	RAVE_SP_EXPECT_ESCAPED_DATA,
+};
+
+/**
+ * struct rave_sp - Device protocol deframer
+ *
+ * @state:  Current state of the deframer
+ * @data:   Buffer used to collect deframed data
+ * @length: Number of bytes de-framed so far
+ */
+struct rave_sp_deframer {
+	enum rave_sp_deframer_state state;
+	unsigned char data[RAVE_SP_RX_BUFFER_SIZE];
+	size_t length;
+};
+
+/**
+ * struct rave_sp_reply - reply as per RAVE device protocol
+ *
+ * @length:	Expected reply length
+ * @data:	Buffer to store reply payload in
+ * @code:	Expected reply code
+ * @ackid:	Expected reply ACK ID
+ * @completion: Successful reply reception completion
+ */
+struct rave_sp_reply {
+	size_t length;
+	void  *data;
+	u8     code;
+	u8     ackid;
+	struct completion received;
+};
+
+/**
+ * struct rave_sp_checksum - Variant specific checksum implementation details
+ *
+ * @length:	Caculated checksum length
+ * @subroutine: Utilized checksum algorithm implementation
+ */
+struct rave_sp_checksum {
+	size_t length;
+	void (*subroutine)(const u8 *, size_t, u8 *);
+};
+
+/**
+ * struct rave_sp_variant_cmds - Variant specific command routines
+ *
+ * @translate:	     Generic to variant specific command mapping routine
+ * @get_boot_source: Pointer to "get boot source" implementation
+ * @set_boot_source: Pointer to "set boot source" implementation
+ */
+struct rave_sp_variant_cmds {
+	int (*translate)(enum rave_sp_command);
+	int (*get_boot_source)(struct rave_sp *);
+	int (*set_boot_source)(struct rave_sp *, u8);
+};
+
+/**
+ * struct rave_sp_variant - RAVE supervisory processor core variant
+ *
+ * @checksum:	Variant specific checksum implementation
+ * @cmd:	Variant specific command pointer table
+ * @init:	Variant specific initialization sequence implementation
+ * @group:	Attribute group for exposed sysfs entries
+ */
+struct rave_sp_variant {
+	const struct rave_sp_checksum *checksum;
+	struct rave_sp_variant_cmds cmd;
+	void (*init)(struct rave_sp *);
+};
+
+/**
+ * struct rave_sp - RAVE supervisory processor core
+ *
+ * @serdev:			Pointer to underlying serdev
+ * @deframer:			Stored state of the protocol deframer
+ * @ackid:			ACK ID used in last reply sent to the device
+ * @bus_lock:			Lock to serialize access to the device
+ * @reply_lock:			Lock protecting @reply
+ * @reply:			Pointer to memory to store reply payload
+ *
+ * @part_number_firmware:	Firmware version
+ * @part_number_bootloader:	Bootloader version
+ * @reset_reason:		Reset reason
+ * @copper_rev_rmb:		Manufacturing version info
+ * @copper_rev_deb:		Manufacturing version info
+ * @silicon_devid:		MCU silicon version info
+ * @silicon_devrev:		MCU silicon version info
+ * @copper_mod_rmb:		Manufacturing version info
+ * @copper_mod_deb:		Manufacturing version info
+ *
+ * @variant:			Device variant specific information
+ * @event_notifier_list:	Input event notification chain
+ * @group:			Attrubute group for exposed sysfs entries
+ *
+ *
+ * part_number_*, reset_reason, copper_*, and silicon_* fields are all
+ * strings retrived from the device during device probing and made
+ * available for later userspace consumption via sysfs
+ *
+ */
+struct rave_sp {
+	struct serdev_device *serdev;
+
+	struct rave_sp_deframer deframer;
+	atomic_t ackid;
+
+	struct mutex bus_lock;
+	struct mutex reply_lock;
+	struct rave_sp_reply *reply;
+
+	const char *part_number_firmware;
+	const char *part_number_bootloader;
+
+	const char *reset_reason;
+	const char *copper_rev_rmb;
+	const char *copper_rev_deb;
+	const char *silicon_devid;
+	const char *silicon_devrev;
+
+	const char *copper_mod_rmb;
+	const char *copper_mod_deb;
+
+	const struct rave_sp_variant *variant;
+
+	struct blocking_notifier_head event_notifier_list;
+
+	struct attribute_group group;
+	struct dentry *debugfs;
+};
+
+struct rave_sp_rsp_status {
+	u8 bl_bytes[6];
+	u8 fw_bytes[6];
+	u8 gs_format;
+} __packed;
+
+static bool rave_sp_id_is_event(u8 code)
+{
+	return (code & 0xF0) == RAVE_SP_EVNT_BASE;
+}
+
+static void rave_sp_unregister_event_notifier(struct device *dev, void *res)
+{
+	struct rave_sp *sp = dev_get_drvdata(dev->parent);
+	struct notifier_block *nb = *(struct notifier_block **)res;
+	struct blocking_notifier_head *bnh = &sp->event_notifier_list;
+
+	WARN_ON(blocking_notifier_chain_unregister(bnh, nb));
+}
+
+int devm_rave_sp_register_event_notifier(struct device *dev,
+					 struct notifier_block *nb)
+{
+	struct rave_sp *sp = dev_get_drvdata(dev->parent);
+	struct notifier_block **rcnb;
+	int ret;
+
+	rcnb = devres_alloc(rave_sp_unregister_event_notifier,
+			    sizeof(*rcnb), GFP_KERNEL);
+	if (!rcnb)
+		return -ENOMEM;
+
+	ret = blocking_notifier_chain_register(&sp->event_notifier_list, nb);
+	if (!ret) {
+		*rcnb = nb;
+		devres_add(dev, rcnb);
+	} else {
+		devres_free(rcnb);
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(devm_rave_sp_register_event_notifier);
+
+static const char *devm_rave_sp_version(struct device *dev, const char *buf)
+{
+	/*
+	 * NOTE: The format string below uses %02d to display u16
+	 * intentionally for the sake of backwards compatibility with
+	 * legacy software.
+	 */
+	return devm_kasprintf(dev, GFP_KERNEL, "%02d%02d%02d.%c%c\n",
+			      buf[0], get_unaligned_le16(&buf[1]),
+			      buf[3], buf[4], buf[5]);
+}
+
+static int rave_sp_get_status(struct rave_sp *sp,
+			      struct rave_sp_rsp_status *status)
+{
+	u8 cmd[] = {
+		[0] = RAVE_SP_CMD_STATUS,
+		[1] = 0
+	};
+	return rave_sp_exec(sp, cmd, sizeof(cmd), status, sizeof(*status));
+}
+
+#define RAVE_SP_ATTR_RO_STRING(name)					\
+	static ssize_t							\
+	name##_show(struct device *dev,					\
+		    struct device_attribute *attr,			\
+		    char *buf)						\
+	{								\
+		struct rave_sp *sp = dev_get_drvdata(dev);		\
+		return sprintf(buf, "%s", sp->name);			\
+	}								\
+	static DEVICE_ATTR_RO(name)
+
+RAVE_SP_ATTR_RO_STRING(reset_reason);
+
+static int rave_sp_i2c_device_status_get(void *context, u64 *result)
+{
+	struct rave_sp *sp = context;
+	u8 status[2];
+	u8 cmd[] = {
+		[0] = RAVE_SP_CMD_GET_I2C_DEVICE_STATUS,
+		[1] = 0
+	};
+	int ret;
+
+	ret = rave_sp_exec(sp, cmd, sizeof(cmd), &status, sizeof(status));
+	if (ret < 0)
+		return ret;
+
+	*result = get_unaligned_le16(status);
+	return 0;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(rave_sp_i2c_device_status,
+			 rave_sp_i2c_device_status_get,
+			 NULL, "%04llx\n");
+
+
+static int rave_sp_rdu1_get_boot_source(struct rave_sp *sp)
+{
+	struct rave_sp_rsp_status status;
+	int ret;
+
+	ret = rave_sp_get_status(sp, &status);
+
+	return (ret < 0) ? ret : (status.gs_format >> 2) & 0x03;
+}
+
+static int rave_sp_rdu1_set_boot_source(struct rave_sp *sp, u8 boot_source)
+{
+	return -ENOTSUPP;
+}
+
+static int rave_sp_common_set_boot_source(struct rave_sp *sp, u8 boot_source)
+{
+	u8 cmd[] = {
+		[0] = RAVE_SP_CMD_BOOT_SOURCE,
+		[1] = 0,
+		[2] = RAVE_SP_BOOT_SOURCE_SET,
+		[3] = boot_source,
+	};
+
+	return rave_sp_exec(sp, cmd, sizeof(cmd), NULL, 0);
+}
+
+static int rave_sp_common_get_boot_source(struct rave_sp *sp)
+{
+	u8 cmd[] = {
+		[0] = RAVE_SP_CMD_BOOT_SOURCE,
+		[1] = 0,
+		[2] = RAVE_SP_BOOT_SOURCE_GET,
+		[3] = 0,
+	};
+	u8 boot_source;
+	int ret;
+
+	ret = rave_sp_exec(sp, cmd, sizeof(cmd),
+			   &boot_source, sizeof(boot_source));
+
+	return (ret < 0) ? ret : boot_source;
+}
+
+static ssize_t boot_source_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct rave_sp *sp = dev_get_drvdata(dev);
+	int ret;
+
+	ret = sp->variant->cmd.get_boot_source(sp);
+
+	return (ret < 0) ? ret : sprintf(buf, "%d\n", ret);
+}
+
+static ssize_t boot_source_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	struct rave_sp *sp = dev_get_drvdata(dev);
+	u8 boot_source;
+	int ret;
+
+	ret = kstrtou8(buf, 0, &boot_source);
+	if (ret)
+		return ret;
+
+	if (boot_source != RAVE_SP_BOOT_SOURCE_SD &&
+	    boot_source != RAVE_SP_BOOT_SOURCE_EMMC &&
+	    boot_source != RAVE_SP_BOOT_SOURCE_NOR)
+		return -EINVAL;
+
+	ret = sp->variant->cmd.set_boot_source(sp, boot_source);
+
+	return (ret < 0) ? ret : count;
+}
+static DEVICE_ATTR_RW(boot_source);
+
+static void devm_rave_sp_sysfs_group_release(struct device *dev, void *res)
+{
+	struct rave_sp *sp = *(struct rave_sp **)res;
+	const struct attribute_group *group = &sp->group;
+	struct kobject *root = &sp->serdev->dev.kobj;
+
+	sysfs_remove_group(root, group);
+}
+
+static int devm_rave_sysfs_create_group(struct rave_sp *sp)
+{
+	struct rave_sp **rcsp;
+	struct device *dev = &sp->serdev->dev;
+	const struct attribute_group *group = &sp->group;
+	struct kobject *root = &dev->kobj;
+	int ret;
+
+	rcsp = devres_alloc(devm_rave_sp_sysfs_group_release,
+			     sizeof(*rcsp), GFP_KERNEL);
+	if (!rcsp)
+		return -ENOMEM;
+
+	ret = sysfs_create_group(root, group);
+	if (!ret) {
+		*rcsp = sp;
+		devres_add(dev, rcsp);
+	} else {
+		devres_free(rcsp);
+	}
+
+	return ret;
+}
+
+#define RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(attr_name)		\
+	static int rave_sp_print_##attr_name(struct seq_file *file,	\
+					     void *data)		\
+	{								\
+		struct rave_sp *sp = dev_get_drvdata(file->private);	\
+									\
+		seq_printf(file, sp->attr_name);			\
+		return 0;						\
+	}
+
+RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(part_number_firmware);
+RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(part_number_bootloader);
+RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(copper_rev_deb);
+RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(copper_rev_rmb);
+RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(copper_mod_deb);
+RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(copper_mod_rmb);
+RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(silicon_devid);
+RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(silicon_devrev);
+
+#define RAVE_SP_DEBUGFS_CREATE_DEVM_SEQFILE(sp, attr_name)		\
+	debugfs_create_devm_seqfile(&sp->serdev->dev, #attr_name,	\
+				    sp->debugfs,			\
+				    rave_sp_print_##attr_name)
+
+static int rave_sp_debugfs_create(struct rave_sp *sp)
+{
+	struct dentry *file;
+
+	sp->debugfs = debugfs_create_dir("rave", NULL);
+	if (!sp->debugfs)
+		return -ENOMEM;
+
+	file = debugfs_create_file("i2c_device_status", 0444,
+				   sp->debugfs, sp,
+				   &rave_sp_i2c_device_status);
+	if (!file)
+		goto error;
+
+	file = RAVE_SP_DEBUGFS_CREATE_DEVM_SEQFILE(sp, part_number_firmware);
+	if (!file)
+		goto error;
+
+	file = RAVE_SP_DEBUGFS_CREATE_DEVM_SEQFILE(sp, part_number_bootloader);
+	if (!file)
+		goto error;
+
+	file = RAVE_SP_DEBUGFS_CREATE_DEVM_SEQFILE(sp, copper_rev_deb);
+	if (!file)
+		goto error;
+
+	file = RAVE_SP_DEBUGFS_CREATE_DEVM_SEQFILE(sp, copper_rev_rmb);
+	if (!file)
+		goto error;
+
+	file = RAVE_SP_DEBUGFS_CREATE_DEVM_SEQFILE(sp, copper_mod_deb);
+	if (!file)
+		goto error;
+
+	file = RAVE_SP_DEBUGFS_CREATE_DEVM_SEQFILE(sp, copper_mod_rmb);
+	if (!file)
+		goto error;
+
+	file = RAVE_SP_DEBUGFS_CREATE_DEVM_SEQFILE(sp, silicon_devrev);
+	if (!file)
+		goto error;
+
+	file = RAVE_SP_DEBUGFS_CREATE_DEVM_SEQFILE(sp, silicon_devid);
+	if (!file)
+		goto error;
+
+	return 0;
+error:
+	debugfs_remove_recursive(sp->debugfs);
+	return -ENOMEM;
+}
+
+static void rave_sp_degugfs_release(struct device *dev, void *res)
+{
+	struct rave_sp *sp = *(struct rave_sp **)res;
+
+	debugfs_remove_recursive(sp->debugfs);
+}
+
+static int devm_rave_sp_debugfs_create(struct rave_sp *sp)
+{
+	struct rave_sp **rcsp;
+	struct device *dev = &sp->serdev->dev;
+	int ret;
+
+	rcsp = devres_alloc(rave_sp_degugfs_release, sizeof(*rcsp), GFP_KERNEL);
+	if (!rcsp)
+		return -ENOMEM;
+
+	ret = rave_sp_debugfs_create(sp);
+	if (!ret) {
+		*rcsp = sp;
+		devres_add(dev, rcsp);
+	} else {
+		devres_free(rcsp);
+	}
+
+	return ret;
+}
+
+static void csum_8b2c(const u8 *buf, size_t size, u8 *crc)
+{
+	*crc = *buf++;
+	size--;
+
+	while (size--)
+		*crc += *buf++;
+
+	*crc = 1 + ~(*crc);
+}
+
+static void csum_ccitt(const u8 *buf, size_t size, u8 *crc)
+{
+	const u16 calculated = crc_ccitt_false(0xffff, buf, size);
+
+	/*
+	 * While the rest of the wire protocol is little-endian,
+	 * CCITT-16 CRC in RDU2 device is sent out in big-endian order.
+	 */
+	put_unaligned_be16(calculated, crc);
+}
+
+static void *stuff(unsigned char *dest, const unsigned char *src, size_t n)
+{
+	while (n--) {
+		const unsigned char byte = *src++;
+
+		switch (byte) {
+		case RAVE_SP_STX:
+		case RAVE_SP_ETX:
+		case RAVE_SP_DLE:
+			*dest++ = RAVE_SP_DLE;
+			/* FALLTHROUGH */
+		default:
+			*dest++ = byte;
+		}
+	}
+
+	return dest;
+}
+
+static int rave_sp_write(struct rave_sp *sp, const u8 *data, u8 data_size)
+{
+	const size_t checksum_length = sp->variant->checksum->length;
+	unsigned char frame[RAVE_SP_TX_BUFFER_SIZE];
+	unsigned char crc[RAVE_SP_CHECKSUM_SIZE];
+	unsigned char *dest = frame;
+	size_t length;
+
+	if (WARN_ON(checksum_length > sizeof(crc)))
+		return -ENOMEM;
+
+	if (WARN_ON(data_size > sizeof(frame)))
+		return -ENOMEM;
+
+	sp->variant->checksum->subroutine(data, data_size, crc);
+
+	*dest++ = RAVE_SP_STX;
+	dest = stuff(dest, data, data_size);
+	dest = stuff(dest, crc, checksum_length);
+	*dest++ = RAVE_SP_ETX;
+
+	length = dest - frame;
+
+	print_hex_dump(KERN_DEBUG, "rave-sp tx: ", DUMP_PREFIX_NONE,
+		       16, 1, frame, length, false);
+
+	return serdev_device_write(sp->serdev, frame, length, HZ);
+}
+
+static u8 rave_sp_reply_code(u8 command)
+{
+	/*
+	 * There isn't a single rule that describes command code ->
+	 * ACK code transformation, but, going through various
+	 * versions of ICDs, there appear to be three distinct groups
+	 * that can be described by simple transformation.
+	 */
+	switch (command) {
+	case 0xA0 ... 0xBE:
+		/*
+		 * Commands implemented by firmware found in RDU1 and
+		 * older devices all seem to obey the following rule
+		 */
+		return command + 0x20;
+	case 0xE0 ... 0xEF:
+		/*
+		 * Events emitted by all versions of the firmare use
+		 * least significant bit to get an ACK code
+		 */
+		return command | 0x01;
+	default:
+		/*
+		 * Commands implemented by firmware found in RDU2 are
+		 * similar to "old" commands, but they use slightly
+		 * different offset
+		 */
+		return command + 0x40;
+	}
+}
+
+int rave_sp_exec(struct rave_sp *sp,
+		 void *__data,  size_t data_size,
+		 void *reply_data, size_t reply_data_size)
+{
+	struct rave_sp_reply reply = {
+		.data     = reply_data,
+		.length   = reply_data_size,
+		.received = COMPLETION_INITIALIZER_ONSTACK(reply.received),
+	};
+	unsigned char *data = __data;
+	int command, ret = 0;
+	u8 ackid;
+
+	command = sp->variant->cmd.translate(data[0]);
+	if (command < 0)
+		return command;
+
+	ackid       = atomic_inc_return(&sp->ackid);
+	reply.ackid = ackid;
+	reply.code  = rave_sp_reply_code((u8)command),
+
+	mutex_lock(&sp->bus_lock);
+
+	mutex_lock(&sp->reply_lock);
+	sp->reply = &reply;
+	mutex_unlock(&sp->reply_lock);
+
+	data[0] = command;
+	data[1] = ackid;
+
+	rave_sp_write(sp, data, data_size);
+
+	if (!wait_for_completion_timeout(&reply.received, HZ)) {
+		dev_err(&sp->serdev->dev, "Command timeout\n");
+		ret = -ETIMEDOUT;
+
+		mutex_lock(&sp->reply_lock);
+		sp->reply = NULL;
+		mutex_unlock(&sp->reply_lock);
+	}
+
+	mutex_unlock(&sp->bus_lock);
+	return ret;
+}
+EXPORT_SYMBOL(rave_sp_exec);
+
+static void rave_sp_receive_event(struct rave_sp *sp,
+				  const unsigned char *data, size_t length)
+{
+	u8 cmd[] = {
+		[0] = rave_sp_reply_code(data[0]),
+		[1] = data[1],
+	};
+
+	rave_sp_write(sp, cmd, sizeof(cmd));
+
+	blocking_notifier_call_chain(&sp->event_notifier_list,
+				     rave_sp_action_pack(data[0], data[2]),
+				     NULL);
+}
+
+static void rave_sp_receive_reply(struct rave_sp *sp,
+				  const unsigned char *data, size_t length)
+{
+	struct device *dev = &sp->serdev->dev;
+	struct rave_sp_reply *reply;
+	const  size_t payload_length = length - 2;
+
+	mutex_lock(&sp->reply_lock);
+	reply = sp->reply;
+
+	if (reply) {
+		if (reply->code == data[0] && reply->ackid == data[1] &&
+		    payload_length >= reply->length) {
+			/*
+			 * We are relying on memcpy(dst, src, 0) to be a no-op
+			 * when handling commands that have a no-payload reply
+			 */
+			memcpy(reply->data, &data[2], reply->length);
+			complete(&reply->received);
+			sp->reply = NULL;
+		} else {
+			dev_err(dev, "Ignoring incorrect reply\n");
+			dev_dbg(dev, "Code:   expected = 0x%08x received = 0x%08x\n",
+				reply->code, data[0]);
+			dev_dbg(dev, "ACK ID: expected = 0x%08x received = 0x%08x\n",
+				reply->ackid, data[1]);
+			dev_dbg(dev, "Length: expected = %zu received = %zu\n",
+				reply->length, payload_length);
+		}
+	}
+
+	mutex_unlock(&sp->reply_lock);
+}
+
+static void rave_sp_receive_frame(struct rave_sp *sp,
+				  const unsigned char *data,
+				  size_t length)
+{
+	const size_t checksum_length = sp->variant->checksum->length;
+	const size_t payload_length  = length - checksum_length;
+	const u8 *crc_reported       = &data[payload_length];
+	struct device *dev           = &sp->serdev->dev;
+	u8 crc_calculated[checksum_length];
+
+	print_hex_dump(KERN_DEBUG, "rave-sp rx: ", DUMP_PREFIX_NONE,
+		       16, 1, data, length, false);
+
+	if (unlikely(length <= checksum_length)) {
+		dev_warn(dev, "Dropping short frame\n");
+		return;
+	}
+
+	sp->variant->checksum->subroutine(data, payload_length,
+					  crc_calculated);
+
+	if (memcmp(crc_calculated, crc_reported, checksum_length)) {
+		dev_warn(dev, "Dropping bad frame\n");
+		return;
+	}
+
+	if (rave_sp_id_is_event(data[0]))
+		rave_sp_receive_event(sp, data, length);
+	else
+		rave_sp_receive_reply(sp, data, length);
+}
+
+static int rave_sp_receive_buf(struct serdev_device *serdev,
+			       const unsigned char *buf, size_t size)
+{
+	struct device *dev  = &serdev->dev;
+	struct rave_sp *sp = dev_get_drvdata(dev);
+	struct rave_sp_deframer *deframer = &sp->deframer;
+	const unsigned char *src = buf;
+	const unsigned char *end = buf + size;
+	bool reset_framer = false;
+
+	while (src < end) {
+		const unsigned char byte = *src++;
+
+		switch (deframer->state) {
+
+		case RAVE_SP_EXPECT_SOF:
+			if (byte == RAVE_SP_STX)
+				deframer->state = RAVE_SP_EXPECT_DATA;
+			continue;
+
+		case RAVE_SP_EXPECT_DATA:
+			switch (byte) {
+			case RAVE_SP_ETX:
+				rave_sp_receive_frame(sp,
+						      deframer->data,
+						      deframer->length);
+				reset_framer = true;
+				break;
+			case RAVE_SP_STX:
+				dev_warn(dev, "Bad frame: STX before ETX\n");
+				reset_framer = true;
+				break;
+			case RAVE_SP_DLE:
+				deframer->state = RAVE_SP_EXPECT_ESCAPED_DATA;
+				continue;
+			}
+
+		case RAVE_SP_EXPECT_ESCAPED_DATA: /* FALLTHROUGH */
+			deframer->data[deframer->length++] = byte;
+
+			if (deframer->length == sizeof(deframer->data)) {
+				dev_warn(dev, "Bad frame: Too long\n");
+				reset_framer = true;
+				break;
+			}
+
+			deframer->state = RAVE_SP_EXPECT_DATA;
+			break;
+		}
+	}
+
+	if (reset_framer) {
+		deframer->state  = RAVE_SP_EXPECT_SOF;
+		deframer->length = 0;
+	}
+
+	return src - buf;
+}
+
+static int rave_sp_rdu1_cmd_translate(enum rave_sp_command command)
+{
+	if (command >= RAVE_SP_CMD_STATUS &&
+	    command <= RAVE_SP_CMD_CONTROL_EVENTS)
+		return command;
+
+	return -EINVAL;
+}
+
+static int rave_sp_rdu2_cmd_translate(enum rave_sp_command command)
+{
+	if (command >= RAVE_SP_CMD_GET_FIRMWARE_VERSION &&
+	    command <= RAVE_SP_CMD_GET_GPIO_STATE)
+		return command;
+
+	if (command == RAVE_SP_CMD_REQ_COPPER_REV) {
+		/*
+		 * As per RDU2 ICD 3.4.47 CMD_GET_COPPER_REV code is
+		 * different from that for RDU1 and it is set to 0x28.
+		 */
+		return 0x28;
+	}
+
+	return rave_sp_rdu1_cmd_translate(command);
+}
+
+static int rave_sp_default_cmd_translate(enum rave_sp_command command)
+{
+	/*
+	 * All of the following command codes were taken from "Table :
+	 * Communications Protocol Message Types" in section 3.3
+	 * "MESSAGE TYPES" of Rave PIC24 ICD.
+	 */
+	switch (command) {
+	case RAVE_SP_CMD_GET_FIRMWARE_VERSION:
+		return 0x11;
+	case RAVE_SP_CMD_GET_BOOTLOADER_VERSION:
+		return 0x12;
+	case RAVE_SP_CMD_BOOT_SOURCE:
+		return 0x14;
+	case RAVE_SP_CMD_SW_WDT:
+		return 0x1C;
+	case RAVE_SP_CMD_RESET:
+		return 0x1E;
+	case RAVE_SP_CMD_RESET_REASON:
+		return 0x1F;
+	default:
+		return -EINVAL;
+	}
+}
+
+static void rave_sp_load_reset_reason(struct rave_sp *sp)
+{
+	struct device *dev = &sp->serdev->dev;
+	u8 cmd[] = {
+		[0] = RAVE_SP_CMD_RESET_REASON,
+		[1] = 0,
+	};
+	u8 reason;
+	int ret;
+
+	ret = rave_sp_exec(sp, cmd, sizeof(cmd), &reason, sizeof(reason));
+	if (ret) {
+		dev_err(dev, "CMD_RESET_REASON failed %d\n", ret);
+		return;
+	}
+
+	sp->reset_reason = devm_kasprintf(dev, GFP_KERNEL, "%02x\n", reason);
+}
+
+static const char *rave_sp_silicon_to_string(struct device *dev, u32 version)
+{
+	return devm_kasprintf(dev, GFP_KERNEL, "%08x\n", version);
+}
+
+static const char *rave_sp_copper_to_string(struct device *dev, uint8_t version)
+{
+	return devm_kasprintf(dev, GFP_KERNEL, "%02x\n", version);
+}
+
+static void rave_sp_load_silicon_rev(struct rave_sp *sp)
+{
+	struct device *dev = &sp->serdev->dev;
+	u8 cmd[] = {
+		[0] = RAVE_SP_CMD_GET_SP_SILICON_REV,
+		[1] = 0
+	};
+	struct {
+		__le32 devid;
+		__le32 devrev;
+	} __packed reply;
+	u32 devid;
+	u32 devrev;
+	int ret;
+
+	ret = rave_sp_exec(sp, cmd, sizeof(cmd), &reply, sizeof(reply));
+	if (ret) {
+		dev_err(dev, "CMD_GET_SP_SILICON_REV failed %d\n", ret);
+		return;
+	}
+
+	devid  = le32_to_cpu(reply.devid);
+	devrev = le32_to_cpu(reply.devrev);
+
+	sp->silicon_devid  = rave_sp_silicon_to_string(dev, devid);
+	sp->silicon_devrev = rave_sp_silicon_to_string(dev, devrev);
+}
+
+static void rave_sp_rdu1_init(struct rave_sp *sp)
+{
+	struct device *dev = &sp->serdev->dev;
+	u8 cmd[] = {
+		[0] = RAVE_SP_CMD_REQ_COPPER_REV,
+		[1] = 0
+	};
+	struct rave_sp_rsp_status status;
+	u8 revision[2];
+	int ret;
+
+	ret = rave_sp_get_status(sp, &status);
+	if (ret) {
+		dev_err(dev, "CMD_STATUS failed %d\n", ret);
+	} else {
+		sp->part_number_firmware =
+			devm_rave_sp_version(dev, status.fw_bytes);
+		sp->part_number_bootloader =
+			devm_rave_sp_version(dev, status.bl_bytes);
+	}
+
+	ret = rave_sp_exec(sp, cmd, sizeof(cmd), &revision, sizeof(revision));
+	if (ret) {
+		dev_err(dev, "CMD_REQ_COPPER_REV failed %d\n", ret);
+		return;
+	}
+
+	sp->copper_rev_rmb = rave_sp_copper_to_string(dev, revision[0]);
+	sp->copper_rev_deb = rave_sp_copper_to_string(dev, revision[1]);
+}
+
+static void rave_sp_common_init(struct rave_sp *sp)
+{
+	struct device *dev = &sp->serdev->dev;
+	u8 version[6];
+	u8 cmd[2];
+	int ret;
+
+	cmd[0] = RAVE_SP_CMD_GET_FIRMWARE_VERSION;
+	ret = rave_sp_exec(sp, cmd, sizeof(cmd), version, sizeof(version));
+	if (ret)
+		dev_warn(dev, "CMD_GET_FIRMWARE_VERSION failed %d\n", ret);
+	else
+		sp->part_number_firmware = devm_rave_sp_version(dev, version);
+
+	cmd[0] = RAVE_SP_CMD_GET_BOOTLOADER_VERSION;
+	ret = rave_sp_exec(sp, cmd, sizeof(cmd), version, sizeof(version));
+	if (ret) {
+		dev_warn(dev, "CMD_GET_BOOTLOADER_VERSION failed %d\n", ret);
+		return;
+	}
+
+	sp->part_number_bootloader = devm_rave_sp_version(dev, version);
+}
+
+static void rave_sp_rdu2_init(struct rave_sp *sp)
+{
+	struct device *dev = &sp->serdev->dev;
+	u8 cmd[] = {
+		[0] = RAVE_SP_CMD_REQ_COPPER_REV,
+		[1] = 0,
+		[2] = RAVE_SP_RDU2_BOARD_TYPE_RMB,
+	};
+	u8 copper_rev;
+	int ret;
+
+	rave_sp_common_init(sp);
+
+	ret = rave_sp_exec(sp, cmd, sizeof(cmd),
+			   &copper_rev, sizeof(copper_rev));
+	if (ret) {
+		dev_warn(dev,
+			 "RAVE_SP_CMD_REQ_COPPER_REV(RMB) failed %d\n", ret);
+	} else {
+		sp->copper_rev_rmb =
+			rave_sp_copper_to_string(dev, copper_rev & 0x1F);
+		sp->copper_mod_rmb =
+			rave_sp_copper_to_string(dev, copper_rev >> 5);
+	}
+
+	cmd[2] = RAVE_SP_RDU2_BOARD_TYPE_DEB;
+
+	ret = rave_sp_exec(sp, cmd, sizeof(cmd),
+			   &copper_rev, sizeof(copper_rev));
+	if (ret) {
+		dev_warn(dev,
+			 "RAVE_SP_CMD_REQ_COPPER_REV(DEB) failed %d\n", ret);
+		return;
+	}
+
+	sp->copper_rev_deb = rave_sp_copper_to_string(dev,
+						      copper_rev & 0x1F);
+	sp->copper_mod_deb = rave_sp_copper_to_string(dev,
+						      copper_rev >> 5);
+}
+
+static struct attribute *rave_sp_attrs[] = {
+	&dev_attr_boot_source.attr,
+	&dev_attr_reset_reason.attr,
+	NULL
+};
+
+static const struct rave_sp_checksum rave_sp_checksum_8b2c = {
+	.length     = 1,
+	.subroutine = csum_8b2c,
+};
+
+static const struct rave_sp_checksum rave_sp_checksum_ccitt = {
+	.length     = 2,
+	.subroutine = csum_ccitt,
+};
+
+static const struct rave_sp_variant rave_sp_legacy = {
+	.checksum = &rave_sp_checksum_8b2c,
+	.cmd = {
+		.translate = rave_sp_default_cmd_translate,
+		.get_boot_source = rave_sp_common_get_boot_source,
+		.set_boot_source = rave_sp_common_set_boot_source,
+	},
+	.init = rave_sp_common_init,
+};
+
+static const struct rave_sp_variant rave_sp_rdu1 = {
+	.checksum = &rave_sp_checksum_8b2c,
+	.cmd = {
+		.translate = rave_sp_rdu1_cmd_translate,
+		.get_boot_source = rave_sp_rdu1_get_boot_source,
+		.set_boot_source = rave_sp_rdu1_set_boot_source,
+	},
+	.init = rave_sp_rdu1_init,
+};
+
+static const struct rave_sp_variant rave_sp_rdu2 = {
+	.checksum = &rave_sp_checksum_ccitt,
+	.cmd = {
+		.translate = rave_sp_rdu2_cmd_translate,
+		.get_boot_source = rave_sp_common_get_boot_source,
+		.set_boot_source = rave_sp_common_set_boot_source,
+	},
+	.init = rave_sp_rdu2_init,
+};
+
+static const struct of_device_id rave_sp_dt_ids[] = {
+	{ .compatible = COMPATIBLE_RAVE_SP_NIU,  .data = &rave_sp_legacy },
+	{ .compatible = COMPATIBLE_RAVE_SP_MEZZ, .data = &rave_sp_legacy },
+	{ .compatible = COMPATIBLE_RAVE_SP_ESB,  .data = &rave_sp_legacy },
+	{ .compatible = COMPATIBLE_RAVE_SP_RDU1, .data = &rave_sp_rdu1   },
+	{ .compatible = COMPATIBLE_RAVE_SP_RDU2, .data = &rave_sp_rdu2   },
+	{ /* sentinel */ }
+};
+
+static const struct serdev_device_ops rave_sp_serdev_device_ops = {
+	.receive_buf  = rave_sp_receive_buf,
+	.write_wakeup = serdev_device_write_wakeup,
+};
+
+static int rave_sp_probe(struct serdev_device *serdev)
+{
+	struct device *dev = &serdev->dev;
+	const char *unknown = "unknown\n";
+	struct rave_sp *sp;
+	u32 baud;
+	int ret;
+
+	if (of_property_read_u32(dev->of_node, "current-speed", &baud)) {
+		dev_err(dev,
+			"'current-speed' is not specified in device node\n");
+		return -EINVAL;
+	}
+
+	sp = devm_kzalloc(dev, sizeof(*sp), GFP_KERNEL);
+	if (!sp)
+		return -ENOMEM;
+
+	sp->serdev = serdev;
+	dev_set_drvdata(dev, sp);
+
+	sp->variant = of_device_get_match_data(dev);
+	if (!sp->variant)
+		return -ENODEV;
+
+	sp->group.attrs = rave_sp_attrs;
+
+	mutex_init(&sp->bus_lock);
+	mutex_init(&sp->reply_lock);
+	BLOCKING_INIT_NOTIFIER_HEAD(&sp->event_notifier_list);
+
+	serdev_device_set_client_ops(serdev, &rave_sp_serdev_device_ops);
+	ret = serdev_device_open(serdev);
+	if (ret)
+		return ret;
+
+	serdev_device_set_baudrate(serdev, baud);
+
+	sp->silicon_devid		= unknown;
+	sp->silicon_devrev		= unknown;
+	sp->copper_rev_deb		= unknown;
+	sp->copper_rev_rmb		= unknown;
+	sp->copper_mod_deb		= unknown;
+	sp->copper_mod_rmb		= unknown;
+	sp->reset_reason		= unknown;
+	sp->part_number_firmware	= unknown;
+	sp->part_number_bootloader	= unknown;
+
+	sp->variant->init(sp);
+
+	/*
+	 * Those strings already have a \n embedded so no need to have
+	 * one in format string.
+	 */
+	dev_info(dev, "Firmware version: %s", sp->part_number_firmware);
+	dev_info(dev, "Bootloader version: %s", sp->part_number_bootloader);
+
+	rave_sp_load_reset_reason(sp);
+	rave_sp_load_silicon_rev(sp);
+
+	ret = devm_rave_sp_debugfs_create(sp);
+	if (ret)
+		goto close_serdev;
+
+	ret = devm_rave_sysfs_create_group(sp);
+	if (ret)
+		goto close_serdev;
+
+	return of_platform_default_populate(dev->of_node, NULL, dev);
+
+close_serdev:
+	serdev_device_close(serdev);
+	return ret;
+}
+
+static void rave_sp_remove(struct serdev_device *serdev)
+{
+	of_platform_depopulate(&serdev->dev);
+	serdev_device_close(serdev);
+}
+
+MODULE_DEVICE_TABLE(of, rave_sp_dt_ids);
+
+static struct serdev_device_driver rave_sp_drv = {
+	.probe			= rave_sp_probe,
+	.remove			= rave_sp_remove,
+	.driver = {
+		.name		= "rave-sp",
+		.of_match_table	= rave_sp_dt_ids,
+	},
+};
+module_serdev_device_driver(rave_sp_drv);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Andrey Vostrikov <andrey.vostrikov@cogentembedded.com>");
+MODULE_AUTHOR("Nikita Yushchenko <nikita.yoush@cogentembedded.com>");
+MODULE_AUTHOR("Andrey Smirnov <andrew.smirnov@gmail.com>");
+MODULE_DESCRIPTION("RAVE SP core driver");
diff --git a/include/linux/rave-sp.h b/include/linux/rave-sp.h
new file mode 100644
index 000000000000..14f9b3177b03
--- /dev/null
+++ b/include/linux/rave-sp.h
@@ -0,0 +1,54 @@
+#ifndef _LINUX_RAVE_SP_H_
+#define _LINUX_RAVE_SP_H_
+
+enum rave_sp_command {
+	RAVE_SP_CMD_GET_FIRMWARE_VERSION	= 0x20,
+	RAVE_SP_CMD_GET_BOOTLOADER_VERSION	= 0x21,
+	RAVE_SP_CMD_BOOT_SOURCE			= 0x26,
+	RAVE_SP_CMD_GET_BOARD_COPPER_REV	= 0x2B,
+	RAVE_SP_CMD_GET_GPIO_STATE		= 0x2F,
+
+	RAVE_SP_CMD_STATUS			= 0xA0,
+	RAVE_SP_CMD_SW_WDT			= 0xA1,
+	RAVE_SP_CMD_PET_WDT			= 0xA2,
+	RAVE_SP_CMD_RESET			= 0xA7,
+	RAVE_SP_CMD_RESET_REASON		= 0xA8,
+
+	RAVE_SP_CMD_REQ_COPPER_REV		= 0xB6,
+	RAVE_SP_CMD_GET_I2C_DEVICE_STATUS	= 0xBA,
+	RAVE_SP_CMD_GET_SP_SILICON_REV		= 0xB9,
+	RAVE_SP_CMD_CONTROL_EVENTS		= 0xBB,
+
+	RAVE_SP_EVNT_BASE			= 0xE0,
+};
+
+struct rave_sp;
+
+static inline unsigned long rave_sp_action_pack(u8 event, u8 value)
+{
+	return ((unsigned long)value << 8) | event;
+}
+
+static inline u8 rave_sp_action_unpack_event(unsigned long action)
+{
+	return action;
+}
+
+static inline u8 rave_sp_action_unpack_value(unsigned long action)
+{
+	return action >> 8;
+}
+
+int rave_sp_exec(struct rave_sp *sp,
+		 void *__data,  size_t data_size,
+		 void *reply_data, size_t reply_data_size);
+int devm_rave_sp_register_event_notifier(struct device *dev,
+				     struct notifier_block *nb);
+
+#define COMPATIBLE_RAVE_SP_NIU		"zii,rave-sp-niu"
+#define COMPATIBLE_RAVE_SP_MEZZ		"zii,rave-sp-mezz"
+#define COMPATIBLE_RAVE_SP_ESB		"zii,rave-sp-esb"
+#define COMPATIBLE_RAVE_SP_RDU1		"zii,rave-sp-rdu1"
+#define COMPATIBLE_RAVE_SP_RDU2		"zii,rave-sp-rdu2"
+
+#endif /* _LINUX_RAVE_SP_H_ */
-- 
2.13.5

[PATCH v6 2/2] dt-bindings: mfd: Add bindings for ZII RAVE devices

[Andrey Smirnov]

Add Device Tree bindings for RAVE Supervisory Processor, an MCU
implementing varoius bits of housekeeping functionality (watchdoging,
backlight control, LED control, etc) on RAVE family of products by
Zodiac Inflight Innovations.

Cc: cphealy@gmail.com
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Pavel Machek <pavel@ucw.cz>
Acked-by: Rob Herring <robh+dt@kernel.org>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 .../devicetree/bindings/mfd/zii,rave-sp.txt        | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/zii,rave-sp.txt

diff --git a/Documentation/devicetree/bindings/mfd/zii,rave-sp.txt b/Documentation/devicetree/bindings/mfd/zii,rave-sp.txt
new file mode 100644
index 000000000000..088eff9ddb78
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/zii,rave-sp.txt
@@ -0,0 +1,39 @@
+Zodiac Inflight Innovations RAVE Supervisory Processor
+
+RAVE Supervisory Processor communicates with SoC over UART. It is
+expected that its Device Tree node is specified as a child of a node
+corresponding to UART controller used for communication.
+
+Required parent device properties:
+
+ - compatible: Should be one of:
+	- "zii,rave-sp-niu"
+	- "zii,rave-sp-mezz"
+	- "zii,rave-sp-esb"
+	- "zii,rave-sp-rdu1"
+	- "zii,rave-sp-rdu2"
+
+ - current-speed: Should be set to baud rate SP device is using
+
+RAVE SP consists of the following sub-devices:
+
+Device				 Description
+------				 -----------
+rave-sp-wdt			: Watchdog
+rave-sp-nvmem			: Interface to onborad EEPROM
+rave-sp-backlight		: Display backlight
+rave-sp-hwmon			: Interface to onboard hardware sensors
+rave-sp-leds			: Interface to onboard LEDs
+rave-sp-input			: Interface to onboard power button
+
+Example of usage:
+
+	rdu {
+		compatible = "zii,rave-sp-rdu2";
+		current-speed = <1000000>;
+
+		watchdog {
+			compatible = "zii,rave-sp-watchdog";
+		};
+	};
+
-- 
2.13.5

Re: [PATCH v6 1/2] platform: Add driver for RAVE Supervisory Processor

[Pavel Machek]

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

Hi!

> Add a driver for RAVE Supervisory Processor, an MCU implementing
> varoius bits of housekeeping functionality (watchdoging, backlight
> control, LED control, etc) on RAVE family of products by Zodiac
> Inflight Innovations.
> 
> This driver implementes core MFD/serdev device as well as
> communication subroutines necessary for commanding the device.

> diff --git a/Documentation/ABI/testing/sysfs-platform-rave-sp b/Documentation/ABI/testing/sysfs-platform-rave-sp
> new file mode 100644
> index 000000000000..81bdc54ba857
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-platform-rave-sp
> @@ -0,0 +1,35 @@
> +What:		/sys/devices/platform/*/serial*/serial*-*/reset_reason
> +Date:		Aug 2017
> +Contact:	"Andrey Smirnov" <andrew.smirnov@gmail.com>
> +Description:    (RO) Indicates reason for last reset.
> +
> +		The following values can be reported:
> +		    * 0  -> Normal Power Off
> +		    * 1  -> Hardware Watchdog
> +		    * 2  -> Software Watchdog
> +		    * 3  -> Input Voltage Out Of Range
> +		    * 4  -> Host Requested
> +		    * 5  -> Temperature Out Of Range
> +		    * 6  -> User Requested (via long Power Button press)
> +		    * 7  -> Illegal Configuration Word
> +		    * 8  -> Illegal Insturction

Typo here.

> +		    * 9  -> Illegal Trap
> +		    * 10 -> Unknown
> +		    * 11 -> Crew Panel Requested

Anyway... If you move management chip to .. I don't know, i2c, the
path would change. Also it would be different path on N900. Userland
should not have to deal with this.

And... this should really be string, as the list will need to grow on
different hardware.

Plus we'll really need better explanations. What is difference between
"normal power off" and "host requested"? 

> +What:		/sys/devices/platform/*/serial*/serial*-*/boot_source
> +Date:		Aug 2017
> +Contact:	"Andrey Smirnov" <andrew.smirnov@gmail.com>
> +Description:    (RW) Indicates currently selected boot source.
> +
> +		The following values are valid:
> +		    * 0  -> SD card
> +		    * 1  -> eMMC
> +		    * 2  -> SPI NOR
> +
> +		NOTE: Setting boot source on RDU1 hardware is
> +		currently not implemented

Same comments apply here.

> +RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(part_number_firmware);
> +RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(part_number_bootloader);
> +RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(copper_rev_deb);
> +RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(copper_rev_rmb);
> +RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(copper_mod_deb);
> +RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(copper_mod_rmb);
> +RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(silicon_devid);
> +RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(silicon_devrev);

Are these going to debugfs?

								Pavel
								
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v6 1/2] platform: Add driver for RAVE Supervisory Processor

[Andrey Smirnov]

On Wed, Aug 30, 2017 at 3:55 AM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> Add a driver for RAVE Supervisory Processor, an MCU implementing
>> varoius bits of housekeeping functionality (watchdoging, backlight
>> control, LED control, etc) on RAVE family of products by Zodiac
>> Inflight Innovations.
>>
>> This driver implementes core MFD/serdev device as well as
>> communication subroutines necessary for commanding the device.
>
>> diff --git a/Documentation/ABI/testing/sysfs-platform-rave-sp b/Documentation/ABI/testing/sysfs-platform-rave-sp
>> new file mode 100644
>> index 000000000000..81bdc54ba857
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-platform-rave-sp
>> @@ -0,0 +1,35 @@
>> +What:                /sys/devices/platform/*/serial*/serial*-*/reset_reason
>> +Date:                Aug 2017
>> +Contact:     "Andrey Smirnov" <andrew.smirnov@gmail.com>
>> +Description:    (RO) Indicates reason for last reset.
>> +
>> +             The following values can be reported:
>> +                 * 0  -> Normal Power Off
>> +                 * 1  -> Hardware Watchdog
>> +                 * 2  -> Software Watchdog
>> +                 * 3  -> Input Voltage Out Of Range
>> +                 * 4  -> Host Requested
>> +                 * 5  -> Temperature Out Of Range
>> +                 * 6  -> User Requested (via long Power Button press)
>> +                 * 7  -> Illegal Configuration Word
>> +                 * 8  -> Illegal Insturction
>
> Typo here.
>
>> +                 * 9  -> Illegal Trap
>> +                 * 10 -> Unknown
>> +                 * 11 -> Crew Panel Requested
>
> Anyway... If you move management chip to .. I don't know, i2c, the
> path would change. Also it would be different path on N900. Userland
> should not have to deal with this.
>
> And... this should really be string, as the list will need to grow on
> different hardware.
>

I think we have a misunderstanding, with this part of the patch set I
am not trying to propose a generic ABI that would be useful for any
other driver but this one. Hence the lack of concern for different
hardware paths (it's not going to change for this device) and device
specific codes instead of generic strings. I can see how my choice of
generic name such as "reset_reason" might suggest that, so I apologize
for any confusion I might have caused. If said generic name is
unacceptable I can change it to "rave_reset_reason" or something
similar and if that is undesirable as well I am happy to drop this
part of the patch and re-visit this later.

> Plus we'll really need better explanations. What is difference between
> "normal power off" and "host requested"?
>

Short answer: I don't know, since this is as much information that ICD
for that device gave me.

Long answer:  It probably can be discerned from the source code of the
firmware/schematic as well as by bothering the right people, but since
I get a feeling that this attribute is not really desirable in its
current from, I'll punt doing that.

>> +What:                /sys/devices/platform/*/serial*/serial*-*/boot_source
>> +Date:                Aug 2017
>> +Contact:     "Andrey Smirnov" <andrew.smirnov@gmail.com>
>> +Description:    (RW) Indicates currently selected boot source.
>> +
>> +             The following values are valid:
>> +                 * 0  -> SD card
>> +                 * 1  -> eMMC
>> +                 * 2  -> SPI NOR
>> +
>> +             NOTE: Setting boot source on RDU1 hardware is
>> +             currently not implemented
>
> Same comments apply here.
>

Yep, same comment for me as well :-)

>> +RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(part_number_firmware);
>> +RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(part_number_bootloader);
>> +RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(copper_rev_deb);
>> +RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(copper_rev_rmb);
>> +RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(copper_mod_deb);
>> +RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(copper_mod_rmb);
>> +RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(silicon_devid);
>> +RAVE_SP_DEFINE_SEQ_FILE_PRINT_FUNCTION(silicon_devrev);
>
> Are these going to debugfs?

They already are there. See rave_sp_debugfs_create() where those
helper functions are being used.

Thanks,
Andrey Smirnov

Re: [PATCH v6 1/2] platform: Add driver for RAVE Supervisory Processor

[Pavel Machek]

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

Hi!

> >> +                 * 9  -> Illegal Trap
> >> +                 * 10 -> Unknown
> >> +                 * 11 -> Crew Panel Requested
> >
> > Anyway... If you move management chip to .. I don't know, i2c, the
> > path would change. Also it would be different path on N900. Userland
> > should not have to deal with this.
> >
> > And... this should really be string, as the list will need to grow on
> > different hardware.
> 
> I think we have a misunderstanding, with this part of the patch set I
> am not trying to propose a generic ABI that would be useful for any
> other driver but this one. Hence the lack of concern for different

Yes, but sorry, that's no-go.  Kernel should hide differences between
different machiens, and it should be rather easy in this case.

> hardware paths (it's not going to change for this device) and device
> specific codes instead of generic strings. I can see how my choice of
> generic name such as "reset_reason" might suggest that, so I apologize
> for any confusion I might have caused. If said generic name is
> unacceptable I can change it to "rave_reset_reason" or something
> similar and if that is undesirable as well I am happy to drop this
> part of the patch and re-visit this later.

Ok, can we do the re-visit thing? You may want to contact these people:

Date: Wed, 30 Aug 2017 12:59:55 +0200
From: Pavel Machek <pavel@ucw.cz>
To: Vadim Pasternak <vadimp@mellanox.com>
Cc: lee.jones@linaro.org, robh+dt@kernel.org,
devicetree@vger.kernel.org,
        j.anaszewski@samsung.com, rpurdie@rpsys.net,
	linux-leds@vger.kernel.org,
	        jiri@resnulli.us, gregkh@linuxfoundation.org,
		platform-driver-x86@vger.kernel.org
Subject: Re: [patch v4 1/2] mfd: Add Mellanox regmap core driver

They are currently trying to add driver, including

>  Cause of reset nodes:
>  - cpu_kernel_panic
>  - cpu_shutdown
>  - bmc_warm_reset

...which sounds very similar to what you are doing.
		
> > Plus we'll really need better explanations. What is difference between
> > "normal power off" and "host requested"?
> >
> 
> Short answer: I don't know, since this is as much information that ICD
> for that device gave me.
> 
> Long answer:  It probably can be discerned from the source code of the
> firmware/schematic as well as by bothering the right people, but since
> I get a feeling that this attribute is not really desirable in its
> current from, I'll punt doing that.

Ok.

> >> +             The following values are valid:
> >> +                 * 0  -> SD card
> >> +                 * 1  -> eMMC
> >> +                 * 2  -> SPI NOR
> >> +
> >> +             NOTE: Setting boot source on RDU1 hardware is
> >> +             currently not implemented
> >
> > Same comments apply here.
> 
> Yep, same comment for me as well :-)

Ok :-).

Best regards,
							Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v6 1/2] platform: Add driver for RAVE Supervisory Processor

[Nikita Yushchenko]

30.08.2017 23:38, Pavel Machek wrote:
> Hi!
> 
>>>> +                 * 9  -> Illegal Trap
>>>> +                 * 10 -> Unknown
>>>> +                 * 11 -> Crew Panel Requested
>>>
>>> Anyway... If you move management chip to .. I don't know, i2c, the
>>> path would change. Also it would be different path on N900. Userland
>>> should not have to deal with this.
>>>
>>> And... this should really be string, as the list will need to grow on
>>> different hardware.
>>
>> I think we have a misunderstanding, with this part of the patch set I
>> am not trying to propose a generic ABI that would be useful for any
>> other driver but this one. Hence the lack of concern for different
> 
> Yes, but sorry, that's no-go.  Kernel should hide differences between
> different machiens, and it should be rather easy in this case.

There is an interest to have reset reason exported on other ZII hardware
as well.

I think that trying to make this generic is purely synthetic. This
information is board-specific per it's nature, it comes from what board
is designed for, different boards have quite different sets of possible
reset reasons. What is needed is - pass this board-specific information
to board-specific user space.

What's proper API for that, if not a sysfs attribute?

Re: [PATCH v6 1/2] platform: Add driver for RAVE Supervisory Processor

[Pavel Machek]

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

On Thu 2017-08-31 11:10:35, Nikita Yushchenko wrote:
> 
> 
> 30.08.2017 23:38, Pavel Machek wrote:
> > Hi!
> > 
> >>>> +                 * 9  -> Illegal Trap
> >>>> +                 * 10 -> Unknown
> >>>> +                 * 11 -> Crew Panel Requested
> >>>
> >>> Anyway... If you move management chip to .. I don't know, i2c, the
> >>> path would change. Also it would be different path on N900. Userland
> >>> should not have to deal with this.
> >>>
> >>> And... this should really be string, as the list will need to grow on
> >>> different hardware.
> >>
> >> I think we have a misunderstanding, with this part of the patch set I
> >> am not trying to propose a generic ABI that would be useful for any
> >> other driver but this one. Hence the lack of concern for different
> > 
> > Yes, but sorry, that's no-go.  Kernel should hide differences between
> > different machiens, and it should be rather easy in this case.
> 
> There is an interest to have reset reason exported on other ZII hardware
> as well.
> 
> I think that trying to make this generic is purely synthetic. This
> information is board-specific per it's nature, it comes from what board
> is designed for, different boards have quite different sets of possible
> reset reasons. What is needed is - pass this board-specific information
> to board-specific user space.
> 
> What's proper API for that, if not a sysfs attribute?

Please go through the thread.

Sysfs attribute is okay, but:

1) it should probably be a string

2) it should certainly be superset of all the reasons

3) it should be in generic place, say /sys/power/reset_reason

4) it should be documented what each state means
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v6 1/2] platform: Add driver for RAVE Supervisory Processor

[Nikita Yushchenko]

>> I think that trying to make this generic is purely synthetic. This
>> information is board-specific per it's nature, it comes from what board
>> is designed for, different boards have quite different sets of possible
>> reset reasons. What is needed is - pass this board-specific information
>> to board-specific user space.
>>
>> What's proper API for that, if not a sysfs attribute?
> 
> Please go through the thread.
> 
> Sysfs attribute is okay, but:
> 
> 1) it should probably be a string
> 
> 2) it should certainly be superset of all the reasons
> 
> 3) it should be in generic place, say /sys/power/reset_reason
> 
> 4) it should be documented what each state means

What I'm concerned here is that a requirement appears for kernel driver
to keep and maintain knowledge of what all that codes mean. For me, it
looks like information locality breakage. Information in question is
application-specific, it definitely changes from board to board and
probably will change for particular board over time.

It's desirable to keep this information application-private, and have
kernel only passed it from hardware to application, unmodified and
uninterpreted.

Requiring kernel driver to interpret application-specific information
only increases complexity (i.e. any changes will have to be maintained
in two places - in driver and in application).

So question is - is there any proper API to communicate
application-private information from hardware through kernel to
userspace without any in-kernel interpretation?

Re: [PATCH v6 1/2] platform: Add driver for RAVE Supervisory Processor

[Pavel Machek]

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

On Thu 2017-08-31 12:01:01, Nikita Yushchenko wrote:
> >> I think that trying to make this generic is purely synthetic. This
> >> information is board-specific per it's nature, it comes from what board
> >> is designed for, different boards have quite different sets of possible
> >> reset reasons. What is needed is - pass this board-specific information
> >> to board-specific user space.
> >>
> >> What's proper API for that, if not a sysfs attribute?
> > 
> > Please go through the thread.
> > 
> > Sysfs attribute is okay, but:
> > 
> > 1) it should probably be a string
> > 
> > 2) it should certainly be superset of all the reasons
> > 
> > 3) it should be in generic place, say /sys/power/reset_reason
> > 
> > 4) it should be documented what each state means
> 
> What I'm concerned here is that a requirement appears for kernel driver
> to keep and maintain knowledge of what all that codes mean. For me,

There's no way around that. Kernel interfaces need to be
documented. If you are passing codes between kernel and application,
_those codes need to be documented_.
 
> So question is - is there any proper API to communicate
> application-private information from hardware through kernel to
> userspace without any in-kernel interpretation?

No.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v6 1/2] platform: Add driver for RAVE Supervisory Processor

[Greg Kroah-Hartman]

On Mon, Aug 28, 2017 at 09:31:30AM -0700, Andrey Smirnov wrote:
> +static int rave_sp_debugfs_create(struct rave_sp *sp)
> +{
> +	struct dentry *file;
> +
> +	sp->debugfs = debugfs_create_dir("rave", NULL);
> +	if (!sp->debugfs)
> +		return -ENOMEM;

Why do you care about the return value of the debugfs function?  Hint,
you don't...

You should never care if debugfs failed, was built in, or succeeded,
your code should just do the same thing always.  Any value returned from
a debugfs call can just be passed to another one, regardless of the
value returned from the first one.

So here, just save it, and don't check it.

> +	file = debugfs_create_file("i2c_device_status", 0444,
> +				   sp->debugfs, sp,
> +				   &rave_sp_i2c_device_status);
> +	if (!file)
> +		goto error;

Nope, you don't care, just make the call.

> +
> +	file = RAVE_SP_DEBUGFS_CREATE_DEVM_SEQFILE(sp, part_number_firmware);
> +	if (!file)
> +		goto error;

Same for all of these.

> +
> +	file = RAVE_SP_DEBUGFS_CREATE_DEVM_SEQFILE(sp, part_number_bootloader);
> +	if (!file)
> +		goto error;
> +
> +	file = RAVE_SP_DEBUGFS_CREATE_DEVM_SEQFILE(sp, copper_rev_deb);
> +	if (!file)
> +		goto error;
> +
> +	file = RAVE_SP_DEBUGFS_CREATE_DEVM_SEQFILE(sp, copper_rev_rmb);
> +	if (!file)
> +		goto error;
> +
> +	file = RAVE_SP_DEBUGFS_CREATE_DEVM_SEQFILE(sp, copper_mod_deb);
> +	if (!file)
> +		goto error;
> +
> +	file = RAVE_SP_DEBUGFS_CREATE_DEVM_SEQFILE(sp, copper_mod_rmb);
> +	if (!file)
> +		goto error;
> +
> +	file = RAVE_SP_DEBUGFS_CREATE_DEVM_SEQFILE(sp, silicon_devrev);
> +	if (!file)
> +		goto error;
> +
> +	file = RAVE_SP_DEBUGFS_CREATE_DEVM_SEQFILE(sp, silicon_devid);
> +	if (!file)
> +		goto error;
> +
> +	return 0;

And your function can not fail, no need for this to return anything.

> +error:
> +	debugfs_remove_recursive(sp->debugfs);
> +	return -ENOMEM;
> +}
> +
> +static void rave_sp_degugfs_release(struct device *dev, void *res)
> +{
> +	struct rave_sp *sp = *(struct rave_sp **)res;
> +
> +	debugfs_remove_recursive(sp->debugfs);
> +}
> +
> +static int devm_rave_sp_debugfs_create(struct rave_sp *sp)
> +{
> +	struct rave_sp **rcsp;
> +	struct device *dev = &sp->serdev->dev;
> +	int ret;
> +
> +	rcsp = devres_alloc(rave_sp_degugfs_release, sizeof(*rcsp), GFP_KERNEL);
> +	if (!rcsp)
> +		return -ENOMEM;
> +
> +	ret = rave_sp_debugfs_create(sp);
> +	if (!ret) {
> +		*rcsp = sp;
> +		devres_add(dev, rcsp);
> +	} else {
> +		devres_free(rcsp);
> +	}

You should not care what debugfs is doing, if it is working or not.  So
no need to check here either.

debugfs was written to make it dirt-simple to use, I don't know why
people keep trying to put some error handling around it :)

thanks,

greg k-h

Re: [PATCH v6 2/2] dt-bindings: mfd: Add bindings for ZII RAVE devices

[Lee Jones]

On Mon, 28 Aug 2017, Andrey Smirnov wrote:

> Add Device Tree bindings for RAVE Supervisory Processor, an MCU
> implementing varoius bits of housekeeping functionality (watchdoging,
> backlight control, LED control, etc) on RAVE family of products by
> Zodiac Inflight Innovations.
> 
> Cc: cphealy@gmail.com
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Pavel Machek <pavel@ucw.cz>
> Acked-by: Rob Herring <robh+dt@kernel.org>
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  .../devicetree/bindings/mfd/zii,rave-sp.txt        | 39 ++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/zii,rave-sp.txt

Applied, thanks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog