[PATCH v8 0/5] ZII RAVE platform driver

[PATCH v8 0/5] ZII RAVE platform driver

[Andrey Smirnov]

Hi everyone,

This patch series is v8 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.

NOTE:

 * This driver is dependent on crc_ccitt_false()
   2da9378d531f8cc6670c7497f20d936b706ab80b in 'linux-next', the patch
   was pulled in by Andrew Morton and is currently avaiting users, so
   this series might have to go in through Andrew's tree

 * Currently the driver is placed in drivers/platform instead of
   drivers/mfd as per request from Lee Jones (Lee, if you changed your
   mind about it, let me know and I'll move the driver back)


Changes since [v7]:

    - Added watchdog driver to the patchset, so it would be easier to
      understand how parent/children drivers are tied together

    - Added serdev patches to implement devm_serdev_device_open() and make .remove optional

    - "Added" missing serdev_device_close() by converting the driver
      to use devm_serdev_device_open()

    - Converted the driver to use devm_of_platform_populate()

    - Removed needless dependency on MFD_CORE

    - Removed dependency on SERIAL_DEV_CTRL_TTYPORT

Changes since [v6]:

    - Patch 2/2 has been applied by Lee so it is no longer a part of the series

    - Removed all sysfs and debugfs attribute to reduce the scope of
      the driver propsed for inclusion. This is not a critical to have
      feature and can be added/discussed later.

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

Feedback is greatly appreciated!

Thanks,
Andrey Smirnov

[v7] lkml.kernel.org/r/20171013061321.31252-2-andrew.smirnov@gmail.com
[v6] lkml.kernel.org/r/20170828163131.24815-2-andrew.smirnov@gmail.com
[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 (5):
  serdev: Make .remove in struct serdev_device_driver optional
  serdev: Introduce devm_serdev_device_open()
  platform: Add driver for RAVE Supervisory Processor
  watchdog: Add RAVE SP watchdog driver
  dt-bindings: watchdog: Add bindings for RAVE SP watchdog driver

 .../bindings/watchdog/zii,rave-sp-wdt.txt          |  10 +
 Documentation/driver-model/devres.txt              |   3 +
 drivers/platform/Kconfig                           |   2 +
 drivers/platform/Makefile                          |   1 +
 drivers/platform/rave/Kconfig                      |  25 +
 drivers/platform/rave/Makefile                     |   1 +
 drivers/platform/rave/rave-sp.c                    | 670 +++++++++++++++++++++
 drivers/tty/serdev/core.c                          |  31 +-
 drivers/watchdog/Kconfig                           |   7 +
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/rave-sp-wdt.c                     | 310 ++++++++++
 include/linux/rave-sp.h                            |  54 ++
 include/linux/serdev.h                             |   1 +
 13 files changed, 1114 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/watchdog/zii,rave-sp-wdt.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 drivers/watchdog/rave-sp-wdt.c
 create mode 100644 include/linux/rave-sp.h

-- 
2.13.5

[PATCH v8 1/5] serdev: Make .remove in struct serdev_device_driver optional

[Andrey Smirnov]

Using devres infrastructure it is possible to wirte a serdev driver
that doesn't have any code that needs to be called as a part of
.remove. Add code to make .remove optional.

Cc: linux-kernel@vger.kernel.org
Cc: linux-serial@vger.kernel.org
Cc: Rob Herring <robh@kernel.org>
Cc: cphealy@gmail.com
Cc: Guenter Roeck <linux@roeck-us.net>
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>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/tty/serdev/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index c68fb3a8ea1c..f500f6a2ca88 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -252,8 +252,8 @@ static int serdev_drv_probe(struct device *dev)
 static int serdev_drv_remove(struct device *dev)
 {
 	const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
-
-	sdrv->remove(to_serdev_device(dev));
+	if (sdrv->remove)
+		sdrv->remove(to_serdev_device(dev));
 	return 0;
 }
 
-- 
2.13.5

[PATCH v8 2/5] serdev: Introduce devm_serdev_device_open()

[Andrey Smirnov]

Add code implementing managed version of serdev_device_open() for
serdev device drivers that "open" the device during driver's lifecycle
only once (e.g. opened in .probe() and closed in .remove()).

Cc: linux-kernel@vger.kernel.org
Cc: linux-serial@vger.kernel.org
Cc: Rob Herring <robh@kernel.org>
Cc: cphealy@gmail.com
Cc: Guenter Roeck <linux@roeck-us.net>
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>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 Documentation/driver-model/devres.txt |  3 +++
 drivers/tty/serdev/core.c             | 27 +++++++++++++++++++++++++++
 include/linux/serdev.h                |  1 +
 3 files changed, 31 insertions(+)

diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index 69f08c0f23a8..e9c6b5cfeec1 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -383,6 +383,9 @@ RESET
   devm_reset_control_get()
   devm_reset_controller_register()
 
+SERDEV
+  devm_serdev_device_open()
+
 SLAVE DMA ENGINE
   devm_acpi_dma_controller_register()
 
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index f500f6a2ca88..b3a785665c6f 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -116,6 +116,33 @@ void serdev_device_close(struct serdev_device *serdev)
 }
 EXPORT_SYMBOL_GPL(serdev_device_close);
 
+static void devm_serdev_device_release(struct device *dev, void *dr)
+{
+	serdev_device_close(*(struct serdev_device **)dr);
+}
+
+int devm_serdev_device_open(struct device *dev, struct serdev_device *serdev)
+{
+	struct serdev_device **dr;
+	int ret;
+
+	dr = devres_alloc(devm_serdev_device_release, sizeof(*dr), GFP_KERNEL);
+	if (!dr)
+		return -ENOMEM;
+
+	ret = serdev_device_open(serdev);
+	if (ret) {
+		devres_free(dr);
+		return ret;
+	}
+
+	*dr = serdev;
+	devres_add(dev, dr);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(devm_serdev_device_open);
+
 void serdev_device_write_wakeup(struct serdev_device *serdev)
 {
 	complete(&serdev->write_comp);
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index e69402d4a8ae..9929063bd45d 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -193,6 +193,7 @@ static inline int serdev_controller_receive_buf(struct serdev_controller *ctrl,
 
 int serdev_device_open(struct serdev_device *);
 void serdev_device_close(struct serdev_device *);
+int devm_serdev_device_open(struct device *, struct serdev_device *);
 unsigned int serdev_device_set_baudrate(struct serdev_device *, unsigned int);
 void serdev_device_set_flow_control(struct serdev_device *, bool);
 int serdev_device_write_buf(struct serdev_device *, const unsigned char *, size_t);
-- 
2.13.5

[PATCH v8 3/5] 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>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Tested-by: Chris Healy <cphealy@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/platform/Kconfig        |   2 +
 drivers/platform/Makefile       |   1 +
 drivers/platform/rave/Kconfig   |  25 ++
 drivers/platform/rave/Makefile  |   1 +
 drivers/platform/rave/rave-sp.c | 670 ++++++++++++++++++++++++++++++++++++++++
 include/linux/rave-sp.h         |  54 ++++
 6 files changed, 753 insertions(+)
 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/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..c0964a531991
--- /dev/null
+++ b/drivers/platform/rave/Kconfig
@@ -0,0 +1,25 @@
+#
+# Platform support for Zodiac RAVE hardware
+#
+
+menuconfig RAVE_PLATFORMS
+	bool "Platform support for Zodiac RAVE hardware"
+	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"
+	depends on SERIAL_DEV_BUS
+	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..2ca7c8cba01f
--- /dev/null
+++ b/drivers/platform/rave/rave-sp.c
@@ -0,0 +1,670 @@
+/*
+ * 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/export.h>
+#include <linux/init.h>
+#include <linux/slab.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
+ *
+ */
+struct rave_sp_variant_cmds {
+	int (*translate)(enum rave_sp_command);
+};
+
+/**
+ * struct rave_sp_variant - RAVE supervisory processor core variant
+ *
+ * @checksum:	Variant specific checksum implementation
+ * @cmd:	Variant specific command pointer table
+ *
+ */
+struct rave_sp_variant {
+	const struct rave_sp_checksum *checksum;
+	struct rave_sp_variant_cmds cmd;
+};
+
+/**
+ * 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
+ *
+ * @variant:			Device variant specific information
+ * @event_notifier_list:	Input event notification chain
+ *
+ */
+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 struct rave_sp_variant *variant;
+
+	struct blocking_notifier_head event_notifier_list;
+};
+
+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 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 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,
+	},
+};
+
+static const struct rave_sp_variant rave_sp_rdu1 = {
+	.checksum = &rave_sp_checksum_8b2c,
+	.cmd = {
+		.translate = rave_sp_rdu1_cmd_translate,
+	},
+};
+
+static const struct rave_sp_variant rave_sp_rdu2 = {
+	.checksum = &rave_sp_checksum_ccitt,
+	.cmd = {
+		.translate = rave_sp_rdu2_cmd_translate,
+	},
+};
+
+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;
+	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;
+
+	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 = devm_serdev_device_open(dev, serdev);
+	if (ret)
+		return ret;
+
+	serdev_device_set_baudrate(serdev, baud);
+
+	return devm_of_platform_populate(dev);
+}
+
+MODULE_DEVICE_TABLE(of, rave_sp_dt_ids);
+
+static struct serdev_device_driver rave_sp_drv = {
+	.probe			= rave_sp_probe,
+	.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 v8 4/5] watchdog: Add RAVE SP watchdog driver

[Andrey Smirnov]

This driver provides access to RAVE SP watchdog functionality.

Cc: linux-kernel@vger.kernel.org
Cc: linux-watchdog@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>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/watchdog/Kconfig       |   7 +
 drivers/watchdog/Makefile      |   1 +
 drivers/watchdog/rave-sp-wdt.c | 310 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 318 insertions(+)
 create mode 100644 drivers/watchdog/rave-sp-wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index c722cbfdc7e6..533a72248cd1 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -223,6 +223,13 @@ config ZIIRAVE_WATCHDOG
 	  To compile this driver as a module, choose M here: the
 	  module will be called ziirave_wdt.
 
+config RAVE_SP_WATCHDOG
+	tristate "RAVE SP Watchdog timer"
+	depends on RAVE_SP_CORE
+	select WATCHDOG_CORE
+	help
+	  Support for the watchdog on RAVE SP device.
+
 # ALPHA Architecture
 
 # ARM Architecture
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 56adf9fa67d0..5c9556c09f6e 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -223,3 +223,4 @@ obj-$(CONFIG_MAX77620_WATCHDOG) += max77620_wdt.o
 obj-$(CONFIG_ZIIRAVE_WATCHDOG) += ziirave_wdt.o
 obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
 obj-$(CONFIG_MENF21BMC_WATCHDOG) += menf21bmc_wdt.o
+obj-$(CONFIG_RAVE_SP_WATCHDOG) += rave-sp-wdt.o
diff --git a/drivers/watchdog/rave-sp-wdt.c b/drivers/watchdog/rave-sp-wdt.c
new file mode 100644
index 000000000000..86fdc7fb1f92
--- /dev/null
+++ b/drivers/watchdog/rave-sp-wdt.c
@@ -0,0 +1,310 @@
+/*
+ *  rave-sp-wdt.c - Watchdog driver preseing in RAVE SP
+ *
+ * Copyright (C) 2017 Zodiac Inflight Innovation
+ *
+ * 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/delay.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/rave-sp.h>
+#include <linux/watchdog.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/reboot.h>
+#include <linux/slab.h>
+
+enum {
+	RAVE_SP_RESET_BYTE = 1,
+	RAVE_SP_RESET_REASON_NORMAL = 0,
+	RAVE_SP_RESET_DELAY_MS = 500,
+};
+
+struct rave_sp_wdt_variant {
+	unsigned int max_timeout;
+	unsigned int min_timeout;
+
+	int (*configure)(struct watchdog_device *);
+	int (*restart)(struct watchdog_device *);
+};
+
+struct rave_sp_wdt {
+	struct watchdog_device wdd;
+	struct rave_sp *sp;
+	const struct rave_sp_wdt_variant *variant;
+	struct notifier_block reboot_notifier;
+};
+
+static struct rave_sp_wdt *to_rave_sp_wdt(struct watchdog_device *wdd)
+{
+	return container_of(wdd, struct rave_sp_wdt, wdd);
+}
+
+static int __rave_sp_wdt_exec(struct watchdog_device *wdd,
+			      void *data,  size_t data_size,
+			      void *reply, size_t reply_size)
+{
+	return rave_sp_exec(to_rave_sp_wdt(wdd)->sp,
+			    data, data_size, reply, reply_size);
+}
+
+static int rave_sp_wdt_exec(struct watchdog_device *wdd, void *data,
+			    size_t data_size)
+{
+	return __rave_sp_wdt_exec(wdd, data, data_size, NULL, 0);
+}
+
+
+static int rave_sp_wdt_legacy_configure(struct watchdog_device *wdd)
+{
+	const bool enable = watchdog_hw_running(wdd);
+	u8 cmd[] = {
+		[0] = RAVE_SP_CMD_SW_WDT,
+		[1] = 0,
+		[2] = 0,
+		[3] = !!enable,
+		[4] = enable ? wdd->timeout : 0,
+	};
+	return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
+}
+
+static int rave_sp_wdt_rdu_configure(struct watchdog_device *wdd)
+{
+	u8 cmd[] = {
+		[0] = RAVE_SP_CMD_SW_WDT,
+		[1] = 0,
+		[2] = watchdog_hw_running(wdd),
+		[3] = (u8) wdd->timeout,
+		[4] = (u8) (wdd->timeout >> 8),
+	};
+	return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
+}
+
+static int rave_sp_wdt_configure(struct watchdog_device *wdd)
+{
+	return to_rave_sp_wdt(wdd)->variant->configure(wdd);
+}
+
+static int rave_sp_wdt_legacy_restart(struct watchdog_device *wdd)
+{
+	u8 cmd[] = {
+		[0] = RAVE_SP_CMD_RESET,
+		[1] = 0,
+		[2] = RAVE_SP_RESET_BYTE
+	};
+	return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
+}
+
+static int rave_sp_wdt_rdu_restart(struct watchdog_device *wdd)
+{
+	u8 cmd[] = {
+		[0] = RAVE_SP_CMD_RESET,
+		[1] = 0,
+		[2] = RAVE_SP_RESET_BYTE,
+		[3] = RAVE_SP_RESET_REASON_NORMAL
+	};
+	return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
+}
+
+static int rave_sp_wdt_reboot_notifier(struct notifier_block *nb,
+				       unsigned long action, void *data)
+{
+	/*
+	 * Restart handler is called in atomic context which means we
+	 * can't commuicate to SP via UART. Luckily for use SP will
+	 * wait 500ms before actually resetting us, so we ask it to do
+	 * so here and let the rest of the system go on wrapping
+	 * things up.
+	 */
+	if (action == SYS_DOWN || action == SYS_HALT) {
+		struct rave_sp_wdt *sp_wd =
+			container_of(nb, struct rave_sp_wdt, reboot_notifier);
+
+		const int ret = sp_wd->variant->restart(&sp_wd->wdd);
+
+		if (ret < 0)
+			dev_err(sp_wd->wdd.parent,
+				"Failed to issue restart command (%d)", ret);
+		return NOTIFY_OK;
+	}
+
+	return NOTIFY_DONE;
+}
+
+static int rave_sp_wdt_restart(struct watchdog_device *wdd,
+			       unsigned long action, void *data)
+{
+	/*
+	 * The actual work was done by reboot notifier above. SP
+	 * firmware waits 500 ms before issuing reset, so let's hang
+	 * here for one second and hopefuly we'd never reach that
+	 * return statement
+	 */
+	mdelay(RAVE_SP_RESET_DELAY_MS);
+	return -EIO;
+}
+
+static int rave_sp_wdt_start(struct watchdog_device *wdd)
+{
+	set_bit(WDOG_HW_RUNNING, &wdd->status);
+	return rave_sp_wdt_configure(wdd);
+}
+
+static int rave_sp_wdt_set_timeout(struct watchdog_device *wdd,
+				   unsigned int timeout)
+{
+	wdd->timeout = timeout;
+	return rave_sp_wdt_configure(wdd);
+}
+
+static int rave_sp_wdt_ping(struct watchdog_device *wdd)
+{
+	u8 cmd[] = {
+		[0] = RAVE_SP_CMD_PET_WDT,
+		[1] = 0,
+	};
+
+	return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
+}
+
+static const struct watchdog_info rave_sp_wdt_info = {
+	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+	.identity = "RAVE SP Watchdog",
+};
+
+static const struct watchdog_ops rave_sp_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = rave_sp_wdt_start,
+	.stop = rave_sp_wdt_configure,
+	.ping = rave_sp_wdt_ping,
+	.set_timeout = rave_sp_wdt_set_timeout,
+	.restart = rave_sp_wdt_restart,
+};
+
+static const struct of_device_id rave_sp_wdt_of_match[] = {
+	{ .compatible = "zii,rave-sp-watchdog" },
+	{}
+};
+
+static const struct rave_sp_wdt_variant rave_sp_wdt_legacy = {
+	.max_timeout = 255,
+	.min_timeout = 1,
+	.configure = rave_sp_wdt_legacy_configure,
+	.restart   = rave_sp_wdt_legacy_restart,
+};
+
+static const struct rave_sp_wdt_variant rave_sp_wdt_rdu = {
+	.max_timeout = 180,
+	.min_timeout = 60,
+	.configure = rave_sp_wdt_rdu_configure,
+	.restart   = rave_sp_wdt_rdu_restart,
+};
+
+static const struct of_device_id rave_sp_wdt_variants[] = {
+	{ .compatible = COMPATIBLE_RAVE_SP_NIU,  .data = &rave_sp_wdt_legacy },
+	{ .compatible = COMPATIBLE_RAVE_SP_MEZZ, .data = &rave_sp_wdt_legacy },
+	{ .compatible = COMPATIBLE_RAVE_SP_ESB,  .data = &rave_sp_wdt_legacy },
+	{ .compatible = COMPATIBLE_RAVE_SP_RDU1, .data = &rave_sp_wdt_rdu    },
+	{ .compatible = COMPATIBLE_RAVE_SP_RDU2, .data = &rave_sp_wdt_rdu    },
+	{ /* sentinel */ }
+};
+
+static int rave_sp_wdt_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct of_device_id *id;
+	struct watchdog_device *wdd;
+	struct rave_sp_wdt *sp_wd;
+	struct nvmem_cell *cell;
+	__le16 timeout = 0;
+	int ret;
+
+	id = of_match_device(rave_sp_wdt_variants, dev->parent);
+	if (WARN_ON(!id))
+		return -ENODEV;
+
+	sp_wd = devm_kzalloc(dev, sizeof(*sp_wd), GFP_KERNEL);
+	if (!sp_wd)
+		return -ENOMEM;
+
+	sp_wd->variant = id->data;
+	sp_wd->sp      = dev_get_drvdata(dev->parent);
+
+	if (WARN_ON(!sp_wd->sp))
+		return -ENODEV;
+
+	wdd              = &sp_wd->wdd;
+	wdd->parent      = dev;
+	wdd->info        = &rave_sp_wdt_info;
+	wdd->ops         = &rave_sp_wdt_ops;
+	wdd->min_timeout = sp_wd->variant->min_timeout;
+	wdd->max_timeout = sp_wd->variant->max_timeout;
+	wdd->status      = WATCHDOG_NOWAYOUT_INIT_STATUS;
+	wdd->timeout     = 60;
+
+	cell = nvmem_cell_get(dev, "wdt_timeout");
+	if (!IS_ERR(cell)) {
+		size_t len;
+		void *value = nvmem_cell_read(cell, &len);
+
+		if (!IS_ERR(value)) {
+			memcpy(&timeout, value, min(len, sizeof(timeout)));
+			kfree(value);
+		}
+		nvmem_cell_put(cell);
+	}
+	watchdog_init_timeout(wdd, le16_to_cpu(timeout), dev);
+	watchdog_set_restart_priority(wdd, 255);
+
+	sp_wd->reboot_notifier.notifier_call = rave_sp_wdt_reboot_notifier;
+	ret = devm_register_reboot_notifier(dev, &sp_wd->reboot_notifier);
+	if (ret) {
+		dev_err(dev, "Failed to register reboot notifier\n");
+		return ret;
+	}
+
+	/*
+	 * We don't know if watchdog is running now. To be sure, let's
+	 * start it and depend on watchdog core to ping it
+	 */
+	wdd->max_hw_heartbeat_ms = wdd->max_timeout * 1000;
+	ret = rave_sp_wdt_start(wdd);
+	if (ret) {
+		dev_err(dev, "Watchdog didn't start\n");
+		return ret;
+	}
+
+	return devm_watchdog_register_device(dev, wdd);
+}
+
+static struct platform_driver rave_sp_wdt_driver = {
+	.probe = rave_sp_wdt_probe,
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = rave_sp_wdt_of_match,
+	},
+};
+
+module_platform_driver(rave_sp_wdt_driver);
+
+MODULE_DEVICE_TABLE(of, rave_sp_wdt_of_match);
+MODULE_LICENSE("GPL");
+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 Watchdog driver");
+MODULE_ALIAS("platform:rave-sp-watchdog");
-- 
2.13.5

[PATCH v8 5/5] dt-bindings: watchdog: Add bindings for RAVE SP watchdog driver

[Andrey Smirnov]

Cc: linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-watchdog@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>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Rob Herring <robh@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 Documentation/devicetree/bindings/watchdog/zii,rave-sp-wdt.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/zii,rave-sp-wdt.txt

diff --git a/Documentation/devicetree/bindings/watchdog/zii,rave-sp-wdt.txt b/Documentation/devicetree/bindings/watchdog/zii,rave-sp-wdt.txt
new file mode 100644
index 000000000000..f242c3eed0dc
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/zii,rave-sp-wdt.txt
@@ -0,0 +1,10 @@
+Zodiac Inflight Innovations RAVE Supervisory Processor Watchdog
+
+Required properties:
+
+ - compatible: Should be "zii,rave-sp-watchdog"
+
+Optional properties:
+
+ - Two byte nvmem cell "wdt_timeout", specified as per
+   Documentation/devicetree/bindings/nvmem/nvmem.txt
-- 
2.13.5

Re: [v8,4/5] watchdog: Add RAVE SP watchdog driver

[Guenter Roeck]

On Wed, Oct 18, 2017 at 10:01:35AM -0700, Andrey Smirnov wrote:
> This driver provides access to RAVE SP watchdog functionality.
> 
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-watchdog@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>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Johan Hovold <johan@kernel.org>
> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  drivers/watchdog/Kconfig       |   7 +
>  drivers/watchdog/Makefile      |   1 +
>  drivers/watchdog/rave-sp-wdt.c | 310 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 318 insertions(+)
>  create mode 100644 drivers/watchdog/rave-sp-wdt.c
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index c722cbfdc7e6..533a72248cd1 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -223,6 +223,13 @@ config ZIIRAVE_WATCHDOG
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ziirave_wdt.
>  
> +config RAVE_SP_WATCHDOG
> +	tristate "RAVE SP Watchdog timer"
> +	depends on RAVE_SP_CORE
> +	select WATCHDOG_CORE
> +	help
> +	  Support for the watchdog on RAVE SP device.
> +
>  # ALPHA Architecture
>  
>  # ARM Architecture
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 56adf9fa67d0..5c9556c09f6e 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -223,3 +223,4 @@ obj-$(CONFIG_MAX77620_WATCHDOG) += max77620_wdt.o
>  obj-$(CONFIG_ZIIRAVE_WATCHDOG) += ziirave_wdt.o
>  obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
>  obj-$(CONFIG_MENF21BMC_WATCHDOG) += menf21bmc_wdt.o
> +obj-$(CONFIG_RAVE_SP_WATCHDOG) += rave-sp-wdt.o
> diff --git a/drivers/watchdog/rave-sp-wdt.c b/drivers/watchdog/rave-sp-wdt.c
> new file mode 100644
> index 000000000000..86fdc7fb1f92
> --- /dev/null
> +++ b/drivers/watchdog/rave-sp-wdt.c
> @@ -0,0 +1,310 @@
> +/*
> + *  rave-sp-wdt.c - Watchdog driver preseing in RAVE SP

present ?

> + *
> + * Copyright (C) 2017 Zodiac Inflight Innovation
> + *
> + * 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/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/rave-sp.h>
> +#include <linux/watchdog.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/reboot.h>
> +#include <linux/slab.h>

Alphabetic order please.

> +
> +enum {
> +	RAVE_SP_RESET_BYTE = 1,
> +	RAVE_SP_RESET_REASON_NORMAL = 0,
> +	RAVE_SP_RESET_DELAY_MS = 500,
> +};
> +
> +struct rave_sp_wdt_variant {
> +	unsigned int max_timeout;
> +	unsigned int min_timeout;
> +
> +	int (*configure)(struct watchdog_device *);
> +	int (*restart)(struct watchdog_device *);
> +};
> +
> +struct rave_sp_wdt {
> +	struct watchdog_device wdd;
> +	struct rave_sp *sp;
> +	const struct rave_sp_wdt_variant *variant;
> +	struct notifier_block reboot_notifier;
> +};
> +
> +static struct rave_sp_wdt *to_rave_sp_wdt(struct watchdog_device *wdd)
> +{
> +	return container_of(wdd, struct rave_sp_wdt, wdd);
> +}
> +
> +static int __rave_sp_wdt_exec(struct watchdog_device *wdd,
> +			      void *data,  size_t data_size,
> +			      void *reply, size_t reply_size)
> +{
> +	return rave_sp_exec(to_rave_sp_wdt(wdd)->sp,
> +			    data, data_size, reply, reply_size);
> +}

This extra function is only called once and thus quite pointless.

> +
> +static int rave_sp_wdt_exec(struct watchdog_device *wdd, void *data,
> +			    size_t data_size)
> +{
> +	return __rave_sp_wdt_exec(wdd, data, data_size, NULL, 0);
> +}
> +
> +

Please run checkpatch --strict and fix what it reports.

> +static int rave_sp_wdt_legacy_configure(struct watchdog_device *wdd)
> +{
> +	const bool enable = watchdog_hw_running(wdd);
> +	u8 cmd[] = {
> +		[0] = RAVE_SP_CMD_SW_WDT,
> +		[1] = 0,
> +		[2] = 0,
> +		[3] = !!enable,

Useless !!. It converts a boolean to a boolean. Besides, isn't it always
true anyway ?

> +		[4] = enable ? wdd->timeout : 0,
> +	};

Interesting; checkpatch doesn't require an empty line after variable
declarations anymore ? Lets keep it anyway for consistency.

> +	return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
> +}
> +
> +static int rave_sp_wdt_rdu_configure(struct watchdog_device *wdd)
> +{
> +	u8 cmd[] = {
> +		[0] = RAVE_SP_CMD_SW_WDT,
> +		[1] = 0,
> +		[2] = watchdog_hw_running(wdd),

Nitpick, but isn't this always true ? There is no stop function,
and HW_RUNNING is set before this function is called.

> +		[3] = (u8) wdd->timeout,
> +		[4] = (u8) (wdd->timeout >> 8),
> +	};
> +	return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
> +}
> +
> +static int rave_sp_wdt_configure(struct watchdog_device *wdd)
> +{
> +	return to_rave_sp_wdt(wdd)->variant->configure(wdd);
> +}
> +
> +static int rave_sp_wdt_legacy_restart(struct watchdog_device *wdd)
> +{
> +	u8 cmd[] = {
> +		[0] = RAVE_SP_CMD_RESET,
> +		[1] = 0,
> +		[2] = RAVE_SP_RESET_BYTE
> +	};
> +	return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
> +}
> +
> +static int rave_sp_wdt_rdu_restart(struct watchdog_device *wdd)
> +{
> +	u8 cmd[] = {
> +		[0] = RAVE_SP_CMD_RESET,
> +		[1] = 0,
> +		[2] = RAVE_SP_RESET_BYTE,
> +		[3] = RAVE_SP_RESET_REASON_NORMAL
> +	};
> +	return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
> +}
> +
> +static int rave_sp_wdt_reboot_notifier(struct notifier_block *nb,
> +				       unsigned long action, void *data)
> +{
> +	/*
> +	 * Restart handler is called in atomic context which means we
> +	 * can't commuicate to SP via UART. Luckily for use SP will

communicate

> +	 * wait 500ms before actually resetting us, so we ask it to do
> +	 * so here and let the rest of the system go on wrapping
> +	 * things up.
> +	 */
> +	if (action == SYS_DOWN || action == SYS_HALT) {
> +		struct rave_sp_wdt *sp_wd =
> +			container_of(nb, struct rave_sp_wdt, reboot_notifier);
> +
> +		const int ret = sp_wd->variant->restart(&sp_wd->wdd);
> +
> +		if (ret < 0)
> +			dev_err(sp_wd->wdd.parent,
> +				"Failed to issue restart command (%d)", ret);
> +		return NOTIFY_OK;
> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static int rave_sp_wdt_restart(struct watchdog_device *wdd,
> +			       unsigned long action, void *data)
> +{
> +	/*
> +	 * The actual work was done by reboot notifier above. SP
> +	 * firmware waits 500 ms before issuing reset, so let's hang
> +	 * here for one second and hopefuly we'd never reach that

RAVE_SP_RESET_DELAY_MS is 500(ms), not one second.

> +	 * return statement
> +	 */
> +	mdelay(RAVE_SP_RESET_DELAY_MS);
> +	return -EIO;
> +}
> +
> +static int rave_sp_wdt_start(struct watchdog_device *wdd)
> +{
> +	set_bit(WDOG_HW_RUNNING, &wdd->status);
> +	return rave_sp_wdt_configure(wdd);
> +}
> +
> +static int rave_sp_wdt_set_timeout(struct watchdog_device *wdd,
> +				   unsigned int timeout)
> +{
> +	wdd->timeout = timeout;
> +	return rave_sp_wdt_configure(wdd);
> +}
> +
> +static int rave_sp_wdt_ping(struct watchdog_device *wdd)
> +{
> +	u8 cmd[] = {
> +		[0] = RAVE_SP_CMD_PET_WDT,
> +		[1] = 0,
> +	};
> +
> +	return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
> +}
> +
> +static const struct watchdog_info rave_sp_wdt_info = {
> +	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> +	.identity = "RAVE SP Watchdog",
> +};
> +
> +static const struct watchdog_ops rave_sp_wdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = rave_sp_wdt_start,
> +	.stop = rave_sp_wdt_configure,
> +	.ping = rave_sp_wdt_ping,
> +	.set_timeout = rave_sp_wdt_set_timeout,
> +	.restart = rave_sp_wdt_restart,
> +};
> +
> +static const struct of_device_id rave_sp_wdt_of_match[] = {
> +	{ .compatible = "zii,rave-sp-watchdog" },
> +	{}
> +};
> +
> +static const struct rave_sp_wdt_variant rave_sp_wdt_legacy = {
> +	.max_timeout = 255,
> +	.min_timeout = 1,
> +	.configure = rave_sp_wdt_legacy_configure,
> +	.restart   = rave_sp_wdt_legacy_restart,
> +};
> +
> +static const struct rave_sp_wdt_variant rave_sp_wdt_rdu = {
> +	.max_timeout = 180,
> +	.min_timeout = 60,
> +	.configure = rave_sp_wdt_rdu_configure,
> +	.restart   = rave_sp_wdt_rdu_restart,
> +};
> +
> +static const struct of_device_id rave_sp_wdt_variants[] = {
> +	{ .compatible = COMPATIBLE_RAVE_SP_NIU,  .data = &rave_sp_wdt_legacy },
> +	{ .compatible = COMPATIBLE_RAVE_SP_MEZZ, .data = &rave_sp_wdt_legacy },
> +	{ .compatible = COMPATIBLE_RAVE_SP_ESB,  .data = &rave_sp_wdt_legacy },
> +	{ .compatible = COMPATIBLE_RAVE_SP_RDU1, .data = &rave_sp_wdt_rdu    },
> +	{ .compatible = COMPATIBLE_RAVE_SP_RDU2, .data = &rave_sp_wdt_rdu    },
> +	{ /* sentinel */ }
> +};
> +
> +static int rave_sp_wdt_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	const struct of_device_id *id;
> +	struct watchdog_device *wdd;
> +	struct rave_sp_wdt *sp_wd;
> +	struct nvmem_cell *cell;
> +	__le16 timeout = 0;
> +	int ret;
> +
> +	id = of_match_device(rave_sp_wdt_variants, dev->parent);
> +	if (WARN_ON(!id))
> +		return -ENODEV;
> +
> +	sp_wd = devm_kzalloc(dev, sizeof(*sp_wd), GFP_KERNEL);
> +	if (!sp_wd)
> +		return -ENOMEM;
> +
> +	sp_wd->variant = id->data;
> +	sp_wd->sp      = dev_get_drvdata(dev->parent);
> +
> +	if (WARN_ON(!sp_wd->sp))
> +		return -ENODEV;
> +
Not sure I understand the value of those tracebacks. Presumably the driver
is instantiated through its parent. Is there precedent that other drivers
in the same situation do this ?

> +	wdd              = &sp_wd->wdd;
> +	wdd->parent      = dev;
> +	wdd->info        = &rave_sp_wdt_info;
> +	wdd->ops         = &rave_sp_wdt_ops;
> +	wdd->min_timeout = sp_wd->variant->min_timeout;
> +	wdd->max_timeout = sp_wd->variant->max_timeout;
> +	wdd->status      = WATCHDOG_NOWAYOUT_INIT_STATUS;
> +	wdd->timeout     = 60;
> +
> +	cell = nvmem_cell_get(dev, "wdt_timeout");
> +	if (!IS_ERR(cell)) {
> +		size_t len;
> +		void *value = nvmem_cell_read(cell, &len);
> +
> +		if (!IS_ERR(value)) {
> +			memcpy(&timeout, value, min(len, sizeof(timeout)));
> +			kfree(value);
> +		}
> +		nvmem_cell_put(cell);
> +	}
> +	watchdog_init_timeout(wdd, le16_to_cpu(timeout), dev);
> +	watchdog_set_restart_priority(wdd, 255);
> +
> +	sp_wd->reboot_notifier.notifier_call = rave_sp_wdt_reboot_notifier;
> +	ret = devm_register_reboot_notifier(dev, &sp_wd->reboot_notifier);
> +	if (ret) {
> +		dev_err(dev, "Failed to register reboot notifier\n");
> +		return ret;
> +	}
> +
> +	/*
> +	 * We don't know if watchdog is running now. To be sure, let's
> +	 * start it and depend on watchdog core to ping it
> +	 */
> +	wdd->max_hw_heartbeat_ms = wdd->max_timeout * 1000;
> +	ret = rave_sp_wdt_start(wdd);
> +	if (ret) {
> +		dev_err(dev, "Watchdog didn't start\n");
> +		return ret;
> +	}
> +
> +	return devm_watchdog_register_device(dev, wdd);
> +}
> +
> +static struct platform_driver rave_sp_wdt_driver = {
> +	.probe = rave_sp_wdt_probe,
> +	.driver = {
> +		.name = KBUILD_MODNAME,
> +		.of_match_table = rave_sp_wdt_of_match,
> +	},
> +};
> +
> +module_platform_driver(rave_sp_wdt_driver);
> +
> +MODULE_DEVICE_TABLE(of, rave_sp_wdt_of_match);
> +MODULE_LICENSE("GPL");
> +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 Watchdog driver");
> +MODULE_ALIAS("platform:rave-sp-watchdog");

Re: [PATCH v8 1/5] serdev: Make .remove in struct serdev_device_driver optional

[Guenter Roeck]

On 10/18/2017 10:01 AM, Andrey Smirnov wrote:
> Using devres infrastructure it is possible to wirte a serdev driver

s/wirte/write/

otherwise

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> that doesn't have any code that needs to be called as a part of
> .remove. Add code to make .remove optional.
> 
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-serial@vger.kernel.org
> Cc: Rob Herring <robh@kernel.org>
> Cc: cphealy@gmail.com
> Cc: Guenter Roeck <linux@roeck-us.net>
> 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>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Johan Hovold <johan@kernel.org>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>   drivers/tty/serdev/core.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index c68fb3a8ea1c..f500f6a2ca88 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -252,8 +252,8 @@ static int serdev_drv_probe(struct device *dev)
>   static int serdev_drv_remove(struct device *dev)
>   {
>   	const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
> -
> -	sdrv->remove(to_serdev_device(dev));
> +	if (sdrv->remove)
> +		sdrv->remove(to_serdev_device(dev));
>   	return 0;
>   }
>   
> 

Re: [PATCH v8 2/5] serdev: Introduce devm_serdev_device_open()

[Guenter Roeck]

On 10/18/2017 10:01 AM, Andrey Smirnov wrote:
> Add code implementing managed version of serdev_device_open() for
> serdev device drivers that "open" the device during driver's lifecycle
> only once (e.g. opened in .probe() and closed in .remove()).
> 
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-serial@vger.kernel.org
> Cc: Rob Herring <robh@kernel.org>
> Cc: cphealy@gmail.com
> Cc: Guenter Roeck <linux@roeck-us.net>
> 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>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Johan Hovold <johan@kernel.org>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   Documentation/driver-model/devres.txt |  3 +++
>   drivers/tty/serdev/core.c             | 27 +++++++++++++++++++++++++++
>   include/linux/serdev.h                |  1 +
>   3 files changed, 31 insertions(+)
> 
> diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
> index 69f08c0f23a8..e9c6b5cfeec1 100644
> --- a/Documentation/driver-model/devres.txt
> +++ b/Documentation/driver-model/devres.txt
> @@ -383,6 +383,9 @@ RESET
>     devm_reset_control_get()
>     devm_reset_controller_register()
>   
> +SERDEV
> +  devm_serdev_device_open()
> +
>   SLAVE DMA ENGINE
>     devm_acpi_dma_controller_register()
>   
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index f500f6a2ca88..b3a785665c6f 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -116,6 +116,33 @@ void serdev_device_close(struct serdev_device *serdev)
>   }
>   EXPORT_SYMBOL_GPL(serdev_device_close);
>   
> +static void devm_serdev_device_release(struct device *dev, void *dr)
> +{
> +	serdev_device_close(*(struct serdev_device **)dr);
> +}
> +
> +int devm_serdev_device_open(struct device *dev, struct serdev_device *serdev)
> +{
> +	struct serdev_device **dr;
> +	int ret;
> +
> +	dr = devres_alloc(devm_serdev_device_release, sizeof(*dr), GFP_KERNEL);
> +	if (!dr)
> +		return -ENOMEM;
> +
> +	ret = serdev_device_open(serdev);
> +	if (ret) {
> +		devres_free(dr);
> +		return ret;
> +	}
> +
> +	*dr = serdev;
> +	devres_add(dev, dr);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(devm_serdev_device_open);
> +
>   void serdev_device_write_wakeup(struct serdev_device *serdev)
>   {
>   	complete(&serdev->write_comp);
> diff --git a/include/linux/serdev.h b/include/linux/serdev.h
> index e69402d4a8ae..9929063bd45d 100644
> --- a/include/linux/serdev.h
> +++ b/include/linux/serdev.h
> @@ -193,6 +193,7 @@ static inline int serdev_controller_receive_buf(struct serdev_controller *ctrl,
>   
>   int serdev_device_open(struct serdev_device *);
>   void serdev_device_close(struct serdev_device *);
> +int devm_serdev_device_open(struct device *, struct serdev_device *);
>   unsigned int serdev_device_set_baudrate(struct serdev_device *, unsigned int);
>   void serdev_device_set_flow_control(struct serdev_device *, bool);
>   int serdev_device_write_buf(struct serdev_device *, const unsigned char *, size_t);
> 

Re: [PATCH v8 3/5] platform: Add driver for RAVE Supervisory Processor

[Guenter Roeck]

On 10/18/2017 10:01 AM, Andrey Smirnov wrote:
> Add a driver for RAVE Supervisory Processor, an MCU implementing
> varoius bits of housekeeping functionality (watchdoging, backlight

various

> 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>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Johan Hovold <johan@kernel.org>
> Tested-by: Chris Healy <cphealy@gmail.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

[question below]

> ---
>   drivers/platform/Kconfig        |   2 +
>   drivers/platform/Makefile       |   1 +
>   drivers/platform/rave/Kconfig   |  25 ++
>   drivers/platform/rave/Makefile  |   1 +
>   drivers/platform/rave/rave-sp.c | 670 ++++++++++++++++++++++++++++++++++++++++
>   include/linux/rave-sp.h         |  54 ++++
>   6 files changed, 753 insertions(+)
>   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/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..c0964a531991
> --- /dev/null
> +++ b/drivers/platform/rave/Kconfig
> @@ -0,0 +1,25 @@
> +#
> +# Platform support for Zodiac RAVE hardware
> +#
> +
> +menuconfig RAVE_PLATFORMS
> +	bool "Platform support for Zodiac RAVE hardware"
> +	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"
> +	depends on SERIAL_DEV_BUS
> +	select CRC_CCITT
> +	help
> +	  Select this to get support for the Supervisory Processor
> +	  device found on several devices in RAVE line of hardware.
> +

Are there going to be more entries in this menu ?

> +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..2ca7c8cba01f
> --- /dev/null
> +++ b/drivers/platform/rave/rave-sp.c
> @@ -0,0 +1,670 @@
> +/*
> + * 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/export.h>
> +#include <linux/init.h>
> +#include <linux/slab.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
> + *
> + */
> +struct rave_sp_variant_cmds {
> +	int (*translate)(enum rave_sp_command);
> +};
> +
> +/**
> + * struct rave_sp_variant - RAVE supervisory processor core variant
> + *
> + * @checksum:	Variant specific checksum implementation
> + * @cmd:	Variant specific command pointer table
> + *
> + */
> +struct rave_sp_variant {
> +	const struct rave_sp_checksum *checksum;
> +	struct rave_sp_variant_cmds cmd;
> +};
> +
> +/**
> + * 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
> + *
> + * @variant:			Device variant specific information
> + * @event_notifier_list:	Input event notification chain
> + *
> + */
> +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 struct rave_sp_variant *variant;
> +
> +	struct blocking_notifier_head event_notifier_list;
> +};
> +
> +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 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 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,
> +	},
> +};
> +
> +static const struct rave_sp_variant rave_sp_rdu1 = {
> +	.checksum = &rave_sp_checksum_8b2c,
> +	.cmd = {
> +		.translate = rave_sp_rdu1_cmd_translate,
> +	},
> +};
> +
> +static const struct rave_sp_variant rave_sp_rdu2 = {
> +	.checksum = &rave_sp_checksum_ccitt,
> +	.cmd = {
> +		.translate = rave_sp_rdu2_cmd_translate,
> +	},
> +};
> +
> +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;
> +	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;
> +
> +	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 = devm_serdev_device_open(dev, serdev);
> +	if (ret)
> +		return ret;
> +
> +	serdev_device_set_baudrate(serdev, baud);
> +
> +	return devm_of_platform_populate(dev);
> +}
> +
> +MODULE_DEVICE_TABLE(of, rave_sp_dt_ids);
> +
> +static struct serdev_device_driver rave_sp_drv = {
> +	.probe			= rave_sp_probe,
> +	.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_ */
> 

Re: [v8,4/5] watchdog: Add RAVE SP watchdog driver

[Joe Perches]

On Sat, 2017-10-21 at 09:47 -0700, Guenter Roeck wrote:
> On Wed, Oct 18, 2017 at 10:01:35AM -0700, Andrey Smirnov wrote:
> > This driver provides access to RAVE SP watchdog functionality.
[]
> > +static int rave_sp_wdt_legacy_configure(struct watchdog_device *wdd)
> > +{
> > +	const bool enable = watchdog_hw_running(wdd);
> > +	u8 cmd[] = {
> > +		[0] = RAVE_SP_CMD_SW_WDT,
> > +		[1] = 0,
> > +		[2] = 0,
> > +		[3] = !!enable,
> 
> Useless !!. It converts a boolean to a boolean. Besides, isn't it always
> true anyway ?

This converts a _Bool to an int which is cast to a u8
The !! is useless though.

> > +		[4] = enable ? wdd->timeout : 0,
> > +	};
> 
> Interesting; checkpatch doesn't require an empty line after variable
> declarations anymore ? Lets keep it anyway for consistency.

checkpatch is primarily a single line oriented parser.
Multi-line declarations aren't parsed.

Re: [v8,4/5] watchdog: Add RAVE SP watchdog driver

[Andrey Smirnov]

On Sat, Oct 21, 2017 at 9:47 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> On Wed, Oct 18, 2017 at 10:01:35AM -0700, Andrey Smirnov wrote:
>> This driver provides access to RAVE SP watchdog functionality.
>>
>> Cc: linux-kernel@vger.kernel.org
>> Cc: linux-watchdog@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>
>> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Cc: Guenter Roeck <linux@roeck-us.net>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Johan Hovold <johan@kernel.org>
>> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> ---
>>  drivers/watchdog/Kconfig       |   7 +
>>  drivers/watchdog/Makefile      |   1 +
>>  drivers/watchdog/rave-sp-wdt.c | 310 +++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 318 insertions(+)
>>  create mode 100644 drivers/watchdog/rave-sp-wdt.c
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index c722cbfdc7e6..533a72248cd1 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -223,6 +223,13 @@ config ZIIRAVE_WATCHDOG
>>         To compile this driver as a module, choose M here: the
>>         module will be called ziirave_wdt.
>>
>> +config RAVE_SP_WATCHDOG
>> +     tristate "RAVE SP Watchdog timer"
>> +     depends on RAVE_SP_CORE
>> +     select WATCHDOG_CORE
>> +     help
>> +       Support for the watchdog on RAVE SP device.
>> +
>>  # ALPHA Architecture
>>
>>  # ARM Architecture
>> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
>> index 56adf9fa67d0..5c9556c09f6e 100644
>> --- a/drivers/watchdog/Makefile
>> +++ b/drivers/watchdog/Makefile
>> @@ -223,3 +223,4 @@ obj-$(CONFIG_MAX77620_WATCHDOG) += max77620_wdt.o
>>  obj-$(CONFIG_ZIIRAVE_WATCHDOG) += ziirave_wdt.o
>>  obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
>>  obj-$(CONFIG_MENF21BMC_WATCHDOG) += menf21bmc_wdt.o
>> +obj-$(CONFIG_RAVE_SP_WATCHDOG) += rave-sp-wdt.o
>> diff --git a/drivers/watchdog/rave-sp-wdt.c b/drivers/watchdog/rave-sp-wdt.c
>> new file mode 100644
>> index 000000000000..86fdc7fb1f92
>> --- /dev/null
>> +++ b/drivers/watchdog/rave-sp-wdt.c
>> @@ -0,0 +1,310 @@
>> +/*
>> + *  rave-sp-wdt.c - Watchdog driver preseing in RAVE SP
>
> present ?
>

Yup, my bad, will fix in v9.

>> + *
>> + * Copyright (C) 2017 Zodiac Inflight Innovation
>> + *
>> + * 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/delay.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/rave-sp.h>
>> +#include <linux/watchdog.h>
>> +#include <linux/nvmem-consumer.h>
>> +#include <linux/reboot.h>
>> +#include <linux/slab.h>
>
> Alphabetic order please.
>

OK, will do.

>> +
>> +enum {
>> +     RAVE_SP_RESET_BYTE = 1,
>> +     RAVE_SP_RESET_REASON_NORMAL = 0,
>> +     RAVE_SP_RESET_DELAY_MS = 500,
>> +};
>> +
>> +struct rave_sp_wdt_variant {
>> +     unsigned int max_timeout;
>> +     unsigned int min_timeout;
>> +
>> +     int (*configure)(struct watchdog_device *);
>> +     int (*restart)(struct watchdog_device *);
>> +};
>> +
>> +struct rave_sp_wdt {
>> +     struct watchdog_device wdd;
>> +     struct rave_sp *sp;
>> +     const struct rave_sp_wdt_variant *variant;
>> +     struct notifier_block reboot_notifier;
>> +};
>> +
>> +static struct rave_sp_wdt *to_rave_sp_wdt(struct watchdog_device *wdd)
>> +{
>> +     return container_of(wdd, struct rave_sp_wdt, wdd);
>> +}
>> +
>> +static int __rave_sp_wdt_exec(struct watchdog_device *wdd,
>> +                           void *data,  size_t data_size,
>> +                           void *reply, size_t reply_size)
>> +{
>> +     return rave_sp_exec(to_rave_sp_wdt(wdd)->sp,
>> +                         data, data_size, reply, reply_size);
>> +}
>
> This extra function is only called once and thus quite pointless.
>

True. Will remove in v9.

>> +
>> +static int rave_sp_wdt_exec(struct watchdog_device *wdd, void *data,
>> +                         size_t data_size)
>> +{
>> +     return __rave_sp_wdt_exec(wdd, data, data_size, NULL, 0);
>> +}
>> +
>> +
>
> Please run checkpatch --strict and fix what it reports.
>

Sure, will do.

>> +static int rave_sp_wdt_legacy_configure(struct watchdog_device *wdd)
>> +{
>> +     const bool enable = watchdog_hw_running(wdd);
>> +     u8 cmd[] = {
>> +             [0] = RAVE_SP_CMD_SW_WDT,
>> +             [1] = 0,
>> +             [2] = 0,
>> +             [3] = !!enable,
>
> Useless !!. It converts a boolean to a boolean. Besides, isn't it always
> true anyway ?

Good point, will remove the "!!". This function also is called as
.stop method (via rave_sp_wdt_configure) for corresponding
"watchdog_device" and I rely on code in watchdog_stop() to clear
WDOG_HW_RUNNING. Is this not a correct thing to do?

>
>> +             [4] = enable ? wdd->timeout : 0,
>> +     };
>
> Interesting; checkpatch doesn't require an empty line after variable
> declarations anymore ? Lets keep it anyway for consistency.
>

OK, will fix in v9.

>> +     return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
>> +}
>> +
>> +static int rave_sp_wdt_rdu_configure(struct watchdog_device *wdd)
>> +{
>> +     u8 cmd[] = {
>> +             [0] = RAVE_SP_CMD_SW_WDT,
>> +             [1] = 0,
>> +             [2] = watchdog_hw_running(wdd),
>
> Nitpick, but isn't this always true ? There is no stop function,
> and HW_RUNNING is set before this function is called.
>

Same as above. Let me know if relying on HW_RUNNING being cleared by
watchdog_stop() is incorrect.

>> +             [3] = (u8) wdd->timeout,
>> +             [4] = (u8) (wdd->timeout >> 8),
>> +     };
>> +     return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
>> +}
>> +
>> +static int rave_sp_wdt_configure(struct watchdog_device *wdd)
>> +{
>> +     return to_rave_sp_wdt(wdd)->variant->configure(wdd);
>> +}
>> +
>> +static int rave_sp_wdt_legacy_restart(struct watchdog_device *wdd)
>> +{
>> +     u8 cmd[] = {
>> +             [0] = RAVE_SP_CMD_RESET,
>> +             [1] = 0,
>> +             [2] = RAVE_SP_RESET_BYTE
>> +     };
>> +     return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
>> +}
>> +
>> +static int rave_sp_wdt_rdu_restart(struct watchdog_device *wdd)
>> +{
>> +     u8 cmd[] = {
>> +             [0] = RAVE_SP_CMD_RESET,
>> +             [1] = 0,
>> +             [2] = RAVE_SP_RESET_BYTE,
>> +             [3] = RAVE_SP_RESET_REASON_NORMAL
>> +     };
>> +     return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
>> +}
>> +
>> +static int rave_sp_wdt_reboot_notifier(struct notifier_block *nb,
>> +                                    unsigned long action, void *data)
>> +{
>> +     /*
>> +      * Restart handler is called in atomic context which means we
>> +      * can't commuicate to SP via UART. Luckily for use SP will
>
> communicate
>

My bad, will fix in v9.

>> +      * wait 500ms before actually resetting us, so we ask it to do
>> +      * so here and let the rest of the system go on wrapping
>> +      * things up.
>> +      */
>> +     if (action == SYS_DOWN || action == SYS_HALT) {
>> +             struct rave_sp_wdt *sp_wd =
>> +                     container_of(nb, struct rave_sp_wdt, reboot_notifier);
>> +
>> +             const int ret = sp_wd->variant->restart(&sp_wd->wdd);
>> +
>> +             if (ret < 0)
>> +                     dev_err(sp_wd->wdd.parent,
>> +                             "Failed to issue restart command (%d)", ret);
>> +             return NOTIFY_OK;
>> +     }
>> +
>> +     return NOTIFY_DONE;
>> +}
>> +
>> +static int rave_sp_wdt_restart(struct watchdog_device *wdd,
>> +                            unsigned long action, void *data)
>> +{
>> +     /*
>> +      * The actual work was done by reboot notifier above. SP
>> +      * firmware waits 500 ms before issuing reset, so let's hang
>> +      * here for one second and hopefuly we'd never reach that
>
> RAVE_SP_RESET_DELAY_MS is 500(ms), not one second.
>

Sorry, meant to wait for twice that delay, hence one second, but wrote
incorrect code. Will fix in v9.

>> +      * return statement
>> +      */
>> +     mdelay(RAVE_SP_RESET_DELAY_MS);
>> +     return -EIO;
>> +}
>> +
>> +static int rave_sp_wdt_start(struct watchdog_device *wdd)
>> +{
>> +     set_bit(WDOG_HW_RUNNING, &wdd->status);
>> +     return rave_sp_wdt_configure(wdd);
>> +}
>> +
>> +static int rave_sp_wdt_set_timeout(struct watchdog_device *wdd,
>> +                                unsigned int timeout)
>> +{
>> +     wdd->timeout = timeout;
>> +     return rave_sp_wdt_configure(wdd);
>> +}
>> +
>> +static int rave_sp_wdt_ping(struct watchdog_device *wdd)
>> +{
>> +     u8 cmd[] = {
>> +             [0] = RAVE_SP_CMD_PET_WDT,
>> +             [1] = 0,
>> +     };
>> +
>> +     return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
>> +}
>> +
>> +static const struct watchdog_info rave_sp_wdt_info = {
>> +     .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
>> +     .identity = "RAVE SP Watchdog",
>> +};
>> +
>> +static const struct watchdog_ops rave_sp_wdt_ops = {
>> +     .owner = THIS_MODULE,
>> +     .start = rave_sp_wdt_start,
>> +     .stop = rave_sp_wdt_configure,
>> +     .ping = rave_sp_wdt_ping,
>> +     .set_timeout = rave_sp_wdt_set_timeout,
>> +     .restart = rave_sp_wdt_restart,
>> +};
>> +
>> +static const struct of_device_id rave_sp_wdt_of_match[] = {
>> +     { .compatible = "zii,rave-sp-watchdog" },
>> +     {}
>> +};
>> +
>> +static const struct rave_sp_wdt_variant rave_sp_wdt_legacy = {
>> +     .max_timeout = 255,
>> +     .min_timeout = 1,
>> +     .configure = rave_sp_wdt_legacy_configure,
>> +     .restart   = rave_sp_wdt_legacy_restart,
>> +};
>> +
>> +static const struct rave_sp_wdt_variant rave_sp_wdt_rdu = {
>> +     .max_timeout = 180,
>> +     .min_timeout = 60,
>> +     .configure = rave_sp_wdt_rdu_configure,
>> +     .restart   = rave_sp_wdt_rdu_restart,
>> +};
>> +
>> +static const struct of_device_id rave_sp_wdt_variants[] = {
>> +     { .compatible = COMPATIBLE_RAVE_SP_NIU,  .data = &rave_sp_wdt_legacy },
>> +     { .compatible = COMPATIBLE_RAVE_SP_MEZZ, .data = &rave_sp_wdt_legacy },
>> +     { .compatible = COMPATIBLE_RAVE_SP_ESB,  .data = &rave_sp_wdt_legacy },
>> +     { .compatible = COMPATIBLE_RAVE_SP_RDU1, .data = &rave_sp_wdt_rdu    },
>> +     { .compatible = COMPATIBLE_RAVE_SP_RDU2, .data = &rave_sp_wdt_rdu    },
>> +     { /* sentinel */ }
>> +};
>> +
>> +static int rave_sp_wdt_probe(struct platform_device *pdev)
>> +{
>> +     struct device *dev = &pdev->dev;
>> +     const struct of_device_id *id;
>> +     struct watchdog_device *wdd;
>> +     struct rave_sp_wdt *sp_wd;
>> +     struct nvmem_cell *cell;
>> +     __le16 timeout = 0;
>> +     int ret;
>> +
>> +     id = of_match_device(rave_sp_wdt_variants, dev->parent);
>> +     if (WARN_ON(!id))
>> +             return -ENODEV;
>> +
>> +     sp_wd = devm_kzalloc(dev, sizeof(*sp_wd), GFP_KERNEL);
>> +     if (!sp_wd)
>> +             return -ENOMEM;
>> +
>> +     sp_wd->variant = id->data;
>> +     sp_wd->sp      = dev_get_drvdata(dev->parent);
>> +
>> +     if (WARN_ON(!sp_wd->sp))
>> +             return -ENODEV;
>> +
> Not sure I understand the value of those tracebacks. Presumably the driver
> is instantiated through its parent. Is there precedent that other drivers
> in the same situation do this ?
>

No precedent. I was just being paranoid. Will remove in v9.

Thanks,
Andrey Smirnov

Re: [PATCH v8 3/5] platform: Add driver for RAVE Supervisory Processor

[Andrey Smirnov]

On Sat, Oct 21, 2017 at 10:17 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> On 10/18/2017 10:01 AM, Andrey Smirnov wrote:
>>
>> Add a driver for RAVE Supervisory Processor, an MCU implementing
>> varoius bits of housekeeping functionality (watchdoging, backlight
>
>
> various
>
>> 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>
>> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Cc: Guenter Roeck <linux@roeck-us.net>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Johan Hovold <johan@kernel.org>
>> Tested-by: Chris Healy <cphealy@gmail.com>
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>
>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>
> [question below]
>
>
>> ---
>>   drivers/platform/Kconfig        |   2 +
>>   drivers/platform/Makefile       |   1 +
>>   drivers/platform/rave/Kconfig   |  25 ++
>>   drivers/platform/rave/Makefile  |   1 +
>>   drivers/platform/rave/rave-sp.c | 670
>> ++++++++++++++++++++++++++++++++++++++++
>>   include/linux/rave-sp.h         |  54 ++++
>>   6 files changed, 753 insertions(+)
>>   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/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..c0964a531991
>> --- /dev/null
>> +++ b/drivers/platform/rave/Kconfig
>> @@ -0,0 +1,25 @@
>> +#
>> +# Platform support for Zodiac RAVE hardware
>> +#
>> +
>> +menuconfig RAVE_PLATFORMS
>> +       bool "Platform support for Zodiac RAVE hardware"
>> +       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"
>> +       depends on SERIAL_DEV_BUS
>> +       select CRC_CCITT
>> +       help
>> +         Select this to get support for the Supervisory Processor
>> +         device found on several devices in RAVE line of hardware.
>> +
>
>
> Are there going to be more entries in this menu ?

Now that you mention it, I don't think so, since all of the MFD cell
devices belong to existing subsystems (watchdog, nvmem, hwmon, leds,
input). Should I just convert it to additional variable for "depends"
instead?

Thanks,
Andrey Smirnov

Re: [v8,4/5] watchdog: Add RAVE SP watchdog driver

[Guenter Roeck]

On Mon, Oct 23, 2017 at 10:01:42AM -0700, Andrey Smirnov wrote:
> On Sat, Oct 21, 2017 at 9:47 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> > On Wed, Oct 18, 2017 at 10:01:35AM -0700, Andrey Smirnov wrote:
> >> This driver provides access to RAVE SP watchdog functionality.
> >>
> >> Cc: linux-kernel@vger.kernel.org
> >> Cc: linux-watchdog@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>
> >> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> >> Cc: Guenter Roeck <linux@roeck-us.net>
> >> Cc: Rob Herring <robh@kernel.org>
> >> Cc: Johan Hovold <johan@kernel.org>
> >> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> >> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> >> ---
> >>  drivers/watchdog/Kconfig       |   7 +
> >>  drivers/watchdog/Makefile      |   1 +
> >>  drivers/watchdog/rave-sp-wdt.c | 310 +++++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 318 insertions(+)
> >>  create mode 100644 drivers/watchdog/rave-sp-wdt.c
> >>
> >> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> >> index c722cbfdc7e6..533a72248cd1 100644
> >> --- a/drivers/watchdog/Kconfig
> >> +++ b/drivers/watchdog/Kconfig
> >> @@ -223,6 +223,13 @@ config ZIIRAVE_WATCHDOG
> >>         To compile this driver as a module, choose M here: the
> >>         module will be called ziirave_wdt.
> >>
> >> +config RAVE_SP_WATCHDOG
> >> +     tristate "RAVE SP Watchdog timer"
> >> +     depends on RAVE_SP_CORE
> >> +     select WATCHDOG_CORE
> >> +     help
> >> +       Support for the watchdog on RAVE SP device.
> >> +
> >>  # ALPHA Architecture
> >>
> >>  # ARM Architecture
> >> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> >> index 56adf9fa67d0..5c9556c09f6e 100644
> >> --- a/drivers/watchdog/Makefile
> >> +++ b/drivers/watchdog/Makefile
> >> @@ -223,3 +223,4 @@ obj-$(CONFIG_MAX77620_WATCHDOG) += max77620_wdt.o
> >>  obj-$(CONFIG_ZIIRAVE_WATCHDOG) += ziirave_wdt.o
> >>  obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
> >>  obj-$(CONFIG_MENF21BMC_WATCHDOG) += menf21bmc_wdt.o
> >> +obj-$(CONFIG_RAVE_SP_WATCHDOG) += rave-sp-wdt.o
> >> diff --git a/drivers/watchdog/rave-sp-wdt.c b/drivers/watchdog/rave-sp-wdt.c
> >> new file mode 100644
> >> index 000000000000..86fdc7fb1f92
> >> --- /dev/null
> >> +++ b/drivers/watchdog/rave-sp-wdt.c
> >> @@ -0,0 +1,310 @@
> >> +/*
> >> + *  rave-sp-wdt.c - Watchdog driver preseing in RAVE SP
> >
> > present ?
> >
> 
> Yup, my bad, will fix in v9.
> 
> >> + *
> >> + * Copyright (C) 2017 Zodiac Inflight Innovation
> >> + *
> >> + * 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/delay.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/module.h>
> >> +#include <linux/of_device.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/rave-sp.h>
> >> +#include <linux/watchdog.h>
> >> +#include <linux/nvmem-consumer.h>
> >> +#include <linux/reboot.h>
> >> +#include <linux/slab.h>
> >
> > Alphabetic order please.
> >
> 
> OK, will do.
> 
> >> +
> >> +enum {
> >> +     RAVE_SP_RESET_BYTE = 1,
> >> +     RAVE_SP_RESET_REASON_NORMAL = 0,
> >> +     RAVE_SP_RESET_DELAY_MS = 500,
> >> +};
> >> +
> >> +struct rave_sp_wdt_variant {
> >> +     unsigned int max_timeout;
> >> +     unsigned int min_timeout;
> >> +
> >> +     int (*configure)(struct watchdog_device *);
> >> +     int (*restart)(struct watchdog_device *);
> >> +};
> >> +
> >> +struct rave_sp_wdt {
> >> +     struct watchdog_device wdd;
> >> +     struct rave_sp *sp;
> >> +     const struct rave_sp_wdt_variant *variant;
> >> +     struct notifier_block reboot_notifier;
> >> +};
> >> +
> >> +static struct rave_sp_wdt *to_rave_sp_wdt(struct watchdog_device *wdd)
> >> +{
> >> +     return container_of(wdd, struct rave_sp_wdt, wdd);
> >> +}
> >> +
> >> +static int __rave_sp_wdt_exec(struct watchdog_device *wdd,
> >> +                           void *data,  size_t data_size,
> >> +                           void *reply, size_t reply_size)
> >> +{
> >> +     return rave_sp_exec(to_rave_sp_wdt(wdd)->sp,
> >> +                         data, data_size, reply, reply_size);
> >> +}
> >
> > This extra function is only called once and thus quite pointless.
> >
> 
> True. Will remove in v9.
> 
> >> +
> >> +static int rave_sp_wdt_exec(struct watchdog_device *wdd, void *data,
> >> +                         size_t data_size)
> >> +{
> >> +     return __rave_sp_wdt_exec(wdd, data, data_size, NULL, 0);
> >> +}
> >> +
> >> +
> >
> > Please run checkpatch --strict and fix what it reports.
> >
> 
> Sure, will do.
> 
> >> +static int rave_sp_wdt_legacy_configure(struct watchdog_device *wdd)
> >> +{
> >> +     const bool enable = watchdog_hw_running(wdd);
> >> +     u8 cmd[] = {
> >> +             [0] = RAVE_SP_CMD_SW_WDT,
> >> +             [1] = 0,
> >> +             [2] = 0,
> >> +             [3] = !!enable,
> >
> > Useless !!. It converts a boolean to a boolean. Besides, isn't it always
> > true anyway ?
> 
> Good point, will remove the "!!". This function also is called as
> .stop method (via rave_sp_wdt_configure) for corresponding
> "watchdog_device" and I rely on code in watchdog_stop() to clear
> WDOG_HW_RUNNING. Is this not a correct thing to do?
> 
> >
> >> +             [4] = enable ? wdd->timeout : 0,
> >> +     };
> >
> > Interesting; checkpatch doesn't require an empty line after variable
> > declarations anymore ? Lets keep it anyway for consistency.
> >
> 
> OK, will fix in v9.
> 
> >> +     return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
> >> +}
> >> +
> >> +static int rave_sp_wdt_rdu_configure(struct watchdog_device *wdd)
> >> +{
> >> +     u8 cmd[] = {
> >> +             [0] = RAVE_SP_CMD_SW_WDT,
> >> +             [1] = 0,
> >> +             [2] = watchdog_hw_running(wdd),
> >
> > Nitpick, but isn't this always true ? There is no stop function,
> > and HW_RUNNING is set before this function is called.
> >
> 
> Same as above. Let me know if relying on HW_RUNNING being cleared by
> watchdog_stop() is incorrect.
> 
Yes, it is incorrect; the knowledge that the flag is cleared before calling
the driver stop function is not part of the API. You should pass that in as
flag.

> >> +             [3] = (u8) wdd->timeout,
> >> +             [4] = (u8) (wdd->timeout >> 8),
> >> +     };
> >> +     return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
> >> +}
> >> +
> >> +static int rave_sp_wdt_configure(struct watchdog_device *wdd)
> >> +{
> >> +     return to_rave_sp_wdt(wdd)->variant->configure(wdd);
> >> +}
> >> +
> >> +static int rave_sp_wdt_legacy_restart(struct watchdog_device *wdd)
> >> +{
> >> +     u8 cmd[] = {
> >> +             [0] = RAVE_SP_CMD_RESET,
> >> +             [1] = 0,
> >> +             [2] = RAVE_SP_RESET_BYTE
> >> +     };
> >> +     return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
> >> +}
> >> +
> >> +static int rave_sp_wdt_rdu_restart(struct watchdog_device *wdd)
> >> +{
> >> +     u8 cmd[] = {
> >> +             [0] = RAVE_SP_CMD_RESET,
> >> +             [1] = 0,
> >> +             [2] = RAVE_SP_RESET_BYTE,
> >> +             [3] = RAVE_SP_RESET_REASON_NORMAL
> >> +     };
> >> +     return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
> >> +}
> >> +
> >> +static int rave_sp_wdt_reboot_notifier(struct notifier_block *nb,
> >> +                                    unsigned long action, void *data)
> >> +{
> >> +     /*
> >> +      * Restart handler is called in atomic context which means we
> >> +      * can't commuicate to SP via UART. Luckily for use SP will
> >
> > communicate
> >
> 
> My bad, will fix in v9.
> 
> >> +      * wait 500ms before actually resetting us, so we ask it to do
> >> +      * so here and let the rest of the system go on wrapping
> >> +      * things up.
> >> +      */
> >> +     if (action == SYS_DOWN || action == SYS_HALT) {
> >> +             struct rave_sp_wdt *sp_wd =
> >> +                     container_of(nb, struct rave_sp_wdt, reboot_notifier);
> >> +
> >> +             const int ret = sp_wd->variant->restart(&sp_wd->wdd);
> >> +
> >> +             if (ret < 0)
> >> +                     dev_err(sp_wd->wdd.parent,
> >> +                             "Failed to issue restart command (%d)", ret);
> >> +             return NOTIFY_OK;
> >> +     }
> >> +
> >> +     return NOTIFY_DONE;
> >> +}
> >> +
> >> +static int rave_sp_wdt_restart(struct watchdog_device *wdd,
> >> +                            unsigned long action, void *data)
> >> +{
> >> +     /*
> >> +      * The actual work was done by reboot notifier above. SP
> >> +      * firmware waits 500 ms before issuing reset, so let's hang
> >> +      * here for one second and hopefuly we'd never reach that
> >
> > RAVE_SP_RESET_DELAY_MS is 500(ms), not one second.
> >
> 
> Sorry, meant to wait for twice that delay, hence one second, but wrote
> incorrect code. Will fix in v9.
> 
> >> +      * return statement
> >> +      */
> >> +     mdelay(RAVE_SP_RESET_DELAY_MS);
> >> +     return -EIO;
> >> +}
> >> +
> >> +static int rave_sp_wdt_start(struct watchdog_device *wdd)
> >> +{
> >> +     set_bit(WDOG_HW_RUNNING, &wdd->status);
> >> +     return rave_sp_wdt_configure(wdd);
> >> +}
> >> +
> >> +static int rave_sp_wdt_set_timeout(struct watchdog_device *wdd,
> >> +                                unsigned int timeout)
> >> +{
> >> +     wdd->timeout = timeout;
> >> +     return rave_sp_wdt_configure(wdd);
> >> +}
> >> +
> >> +static int rave_sp_wdt_ping(struct watchdog_device *wdd)
> >> +{
> >> +     u8 cmd[] = {
> >> +             [0] = RAVE_SP_CMD_PET_WDT,
> >> +             [1] = 0,
> >> +     };
> >> +
> >> +     return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
> >> +}
> >> +
> >> +static const struct watchdog_info rave_sp_wdt_info = {
> >> +     .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> >> +     .identity = "RAVE SP Watchdog",
> >> +};
> >> +
> >> +static const struct watchdog_ops rave_sp_wdt_ops = {
> >> +     .owner = THIS_MODULE,
> >> +     .start = rave_sp_wdt_start,
> >> +     .stop = rave_sp_wdt_configure,
> >> +     .ping = rave_sp_wdt_ping,
> >> +     .set_timeout = rave_sp_wdt_set_timeout,
> >> +     .restart = rave_sp_wdt_restart,
> >> +};
> >> +
> >> +static const struct of_device_id rave_sp_wdt_of_match[] = {
> >> +     { .compatible = "zii,rave-sp-watchdog" },
> >> +     {}
> >> +};
> >> +
> >> +static const struct rave_sp_wdt_variant rave_sp_wdt_legacy = {
> >> +     .max_timeout = 255,
> >> +     .min_timeout = 1,
> >> +     .configure = rave_sp_wdt_legacy_configure,
> >> +     .restart   = rave_sp_wdt_legacy_restart,
> >> +};
> >> +
> >> +static const struct rave_sp_wdt_variant rave_sp_wdt_rdu = {
> >> +     .max_timeout = 180,
> >> +     .min_timeout = 60,
> >> +     .configure = rave_sp_wdt_rdu_configure,
> >> +     .restart   = rave_sp_wdt_rdu_restart,
> >> +};
> >> +
> >> +static const struct of_device_id rave_sp_wdt_variants[] = {
> >> +     { .compatible = COMPATIBLE_RAVE_SP_NIU,  .data = &rave_sp_wdt_legacy },
> >> +     { .compatible = COMPATIBLE_RAVE_SP_MEZZ, .data = &rave_sp_wdt_legacy },
> >> +     { .compatible = COMPATIBLE_RAVE_SP_ESB,  .data = &rave_sp_wdt_legacy },
> >> +     { .compatible = COMPATIBLE_RAVE_SP_RDU1, .data = &rave_sp_wdt_rdu    },
> >> +     { .compatible = COMPATIBLE_RAVE_SP_RDU2, .data = &rave_sp_wdt_rdu    },
> >> +     { /* sentinel */ }
> >> +};
> >> +
> >> +static int rave_sp_wdt_probe(struct platform_device *pdev)
> >> +{
> >> +     struct device *dev = &pdev->dev;
> >> +     const struct of_device_id *id;
> >> +     struct watchdog_device *wdd;
> >> +     struct rave_sp_wdt *sp_wd;
> >> +     struct nvmem_cell *cell;
> >> +     __le16 timeout = 0;
> >> +     int ret;
> >> +
> >> +     id = of_match_device(rave_sp_wdt_variants, dev->parent);
> >> +     if (WARN_ON(!id))
> >> +             return -ENODEV;
> >> +
> >> +     sp_wd = devm_kzalloc(dev, sizeof(*sp_wd), GFP_KERNEL);
> >> +     if (!sp_wd)
> >> +             return -ENOMEM;
> >> +
> >> +     sp_wd->variant = id->data;
> >> +     sp_wd->sp      = dev_get_drvdata(dev->parent);
> >> +
> >> +     if (WARN_ON(!sp_wd->sp))
> >> +             return -ENODEV;
> >> +
> > Not sure I understand the value of those tracebacks. Presumably the driver
> > is instantiated through its parent. Is there precedent that other drivers
> > in the same situation do this ?
> >
> 
> No precedent. I was just being paranoid. Will remove in v9.
> 
> Thanks,
> Andrey Smirnov

Re: [v8,4/5] watchdog: Add RAVE SP watchdog driver

[Andrey Smirnov]

On Mon, Oct 23, 2017 at 3:04 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> On Mon, Oct 23, 2017 at 10:01:42AM -0700, Andrey Smirnov wrote:
>> On Sat, Oct 21, 2017 at 9:47 AM, Guenter Roeck <linux@roeck-us.net> wrote:
>> > On Wed, Oct 18, 2017 at 10:01:35AM -0700, Andrey Smirnov wrote:
>> >> This driver provides access to RAVE SP watchdog functionality.
>> >>
>> >> Cc: linux-kernel@vger.kernel.org
>> >> Cc: linux-watchdog@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>
>> >> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
>> >> Cc: Guenter Roeck <linux@roeck-us.net>
>> >> Cc: Rob Herring <robh@kernel.org>
>> >> Cc: Johan Hovold <johan@kernel.org>
>> >> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
>> >> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> >> ---
>> >>  drivers/watchdog/Kconfig       |   7 +
>> >>  drivers/watchdog/Makefile      |   1 +
>> >>  drivers/watchdog/rave-sp-wdt.c | 310 +++++++++++++++++++++++++++++++++++++++++
>> >>  3 files changed, 318 insertions(+)
>> >>  create mode 100644 drivers/watchdog/rave-sp-wdt.c
>> >>
>> >> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> >> index c722cbfdc7e6..533a72248cd1 100644
>> >> --- a/drivers/watchdog/Kconfig
>> >> +++ b/drivers/watchdog/Kconfig
>> >> @@ -223,6 +223,13 @@ config ZIIRAVE_WATCHDOG
>> >>         To compile this driver as a module, choose M here: the
>> >>         module will be called ziirave_wdt.
>> >>
>> >> +config RAVE_SP_WATCHDOG
>> >> +     tristate "RAVE SP Watchdog timer"
>> >> +     depends on RAVE_SP_CORE
>> >> +     select WATCHDOG_CORE
>> >> +     help
>> >> +       Support for the watchdog on RAVE SP device.
>> >> +
>> >>  # ALPHA Architecture
>> >>
>> >>  # ARM Architecture
>> >> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
>> >> index 56adf9fa67d0..5c9556c09f6e 100644
>> >> --- a/drivers/watchdog/Makefile
>> >> +++ b/drivers/watchdog/Makefile
>> >> @@ -223,3 +223,4 @@ obj-$(CONFIG_MAX77620_WATCHDOG) += max77620_wdt.o
>> >>  obj-$(CONFIG_ZIIRAVE_WATCHDOG) += ziirave_wdt.o
>> >>  obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
>> >>  obj-$(CONFIG_MENF21BMC_WATCHDOG) += menf21bmc_wdt.o
>> >> +obj-$(CONFIG_RAVE_SP_WATCHDOG) += rave-sp-wdt.o
>> >> diff --git a/drivers/watchdog/rave-sp-wdt.c b/drivers/watchdog/rave-sp-wdt.c
>> >> new file mode 100644
>> >> index 000000000000..86fdc7fb1f92
>> >> --- /dev/null
>> >> +++ b/drivers/watchdog/rave-sp-wdt.c
>> >> @@ -0,0 +1,310 @@
>> >> +/*
>> >> + *  rave-sp-wdt.c - Watchdog driver preseing in RAVE SP
>> >
>> > present ?
>> >
>>
>> Yup, my bad, will fix in v9.
>>
>> >> + *
>> >> + * Copyright (C) 2017 Zodiac Inflight Innovation
>> >> + *
>> >> + * 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/delay.h>
>> >> +#include <linux/kernel.h>
>> >> +#include <linux/module.h>
>> >> +#include <linux/of_device.h>
>> >> +#include <linux/platform_device.h>
>> >> +#include <linux/rave-sp.h>
>> >> +#include <linux/watchdog.h>
>> >> +#include <linux/nvmem-consumer.h>
>> >> +#include <linux/reboot.h>
>> >> +#include <linux/slab.h>
>> >
>> > Alphabetic order please.
>> >
>>
>> OK, will do.
>>
>> >> +
>> >> +enum {
>> >> +     RAVE_SP_RESET_BYTE = 1,
>> >> +     RAVE_SP_RESET_REASON_NORMAL = 0,
>> >> +     RAVE_SP_RESET_DELAY_MS = 500,
>> >> +};
>> >> +
>> >> +struct rave_sp_wdt_variant {
>> >> +     unsigned int max_timeout;
>> >> +     unsigned int min_timeout;
>> >> +
>> >> +     int (*configure)(struct watchdog_device *);
>> >> +     int (*restart)(struct watchdog_device *);
>> >> +};
>> >> +
>> >> +struct rave_sp_wdt {
>> >> +     struct watchdog_device wdd;
>> >> +     struct rave_sp *sp;
>> >> +     const struct rave_sp_wdt_variant *variant;
>> >> +     struct notifier_block reboot_notifier;
>> >> +};
>> >> +
>> >> +static struct rave_sp_wdt *to_rave_sp_wdt(struct watchdog_device *wdd)
>> >> +{
>> >> +     return container_of(wdd, struct rave_sp_wdt, wdd);
>> >> +}
>> >> +
>> >> +static int __rave_sp_wdt_exec(struct watchdog_device *wdd,
>> >> +                           void *data,  size_t data_size,
>> >> +                           void *reply, size_t reply_size)
>> >> +{
>> >> +     return rave_sp_exec(to_rave_sp_wdt(wdd)->sp,
>> >> +                         data, data_size, reply, reply_size);
>> >> +}
>> >
>> > This extra function is only called once and thus quite pointless.
>> >
>>
>> True. Will remove in v9.
>>
>> >> +
>> >> +static int rave_sp_wdt_exec(struct watchdog_device *wdd, void *data,
>> >> +                         size_t data_size)
>> >> +{
>> >> +     return __rave_sp_wdt_exec(wdd, data, data_size, NULL, 0);
>> >> +}
>> >> +
>> >> +
>> >
>> > Please run checkpatch --strict and fix what it reports.
>> >
>>
>> Sure, will do.
>>
>> >> +static int rave_sp_wdt_legacy_configure(struct watchdog_device *wdd)
>> >> +{
>> >> +     const bool enable = watchdog_hw_running(wdd);
>> >> +     u8 cmd[] = {
>> >> +             [0] = RAVE_SP_CMD_SW_WDT,
>> >> +             [1] = 0,
>> >> +             [2] = 0,
>> >> +             [3] = !!enable,
>> >
>> > Useless !!. It converts a boolean to a boolean. Besides, isn't it always
>> > true anyway ?
>>
>> Good point, will remove the "!!". This function also is called as
>> .stop method (via rave_sp_wdt_configure) for corresponding
>> "watchdog_device" and I rely on code in watchdog_stop() to clear
>> WDOG_HW_RUNNING. Is this not a correct thing to do?
>>
>> >
>> >> +             [4] = enable ? wdd->timeout : 0,
>> >> +     };
>> >
>> > Interesting; checkpatch doesn't require an empty line after variable
>> > declarations anymore ? Lets keep it anyway for consistency.
>> >
>>
>> OK, will fix in v9.
>>
>> >> +     return rave_sp_wdt_exec(wdd, cmd, sizeof(cmd));
>> >> +}
>> >> +
>> >> +static int rave_sp_wdt_rdu_configure(struct watchdog_device *wdd)
>> >> +{
>> >> +     u8 cmd[] = {
>> >> +             [0] = RAVE_SP_CMD_SW_WDT,
>> >> +             [1] = 0,
>> >> +             [2] = watchdog_hw_running(wdd),
>> >
>> > Nitpick, but isn't this always true ? There is no stop function,
>> > and HW_RUNNING is set before this function is called.
>> >
>>
>> Same as above. Let me know if relying on HW_RUNNING being cleared by
>> watchdog_stop() is incorrect.
>>
> Yes, it is incorrect; the knowledge that the flag is cleared before calling
> the driver stop function is not part of the API. You should pass that in as
> flag.

Good to know. Will fix in v9, then.

Thanks,
Andrey Smirnov

Re: [PATCH v8 5/5] dt-bindings: watchdog: Add bindings for RAVE SP watchdog driver

[Rob Herring]

On Wed, Oct 18, 2017 at 10:01:36AM -0700, Andrey Smirnov wrote:

Commit msg?

> Cc: linux-kernel@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-watchdog@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>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Johan Hovold <johan@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  Documentation/devicetree/bindings/watchdog/zii,rave-sp-wdt.txt | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/zii,rave-sp-wdt.txt
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/zii,rave-sp-wdt.txt b/Documentation/devicetree/bindings/watchdog/zii,rave-sp-wdt.txt
> new file mode 100644
> index 000000000000..f242c3eed0dc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/zii,rave-sp-wdt.txt
> @@ -0,0 +1,10 @@
> +Zodiac Inflight Innovations RAVE Supervisory Processor Watchdog
> +
> +Required properties:
> +
> + - compatible: Should be "zii,rave-sp-watchdog"
> +

Must be a child of ? defined in ?

> +Optional properties:
> +
> + - Two byte nvmem cell "wdt_timeout", specified as per
> +   Documentation/devicetree/bindings/nvmem/nvmem.txt

prop name: definition... is the normal format.

Also, don't use '_' in property names.

Rob

Re: [PATCH v8 5/5] dt-bindings: watchdog: Add bindings for RAVE SP watchdog driver

[Andrey Smirnov]

On Tue, Oct 24, 2017 at 12:07 PM, Rob Herring <robh@kernel.org> wrote:
> On Wed, Oct 18, 2017 at 10:01:36AM -0700, Andrey Smirnov wrote:
>
> Commit msg?
>

OK, will add in v9.

>> Cc: linux-kernel@vger.kernel.org
>> Cc: devicetree@vger.kernel.org
>> Cc: linux-watchdog@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>
>> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Cc: Guenter Roeck <linux@roeck-us.net>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Johan Hovold <johan@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> ---
>>  Documentation/devicetree/bindings/watchdog/zii,rave-sp-wdt.txt | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/watchdog/zii,rave-sp-wdt.txt
>>
>> diff --git a/Documentation/devicetree/bindings/watchdog/zii,rave-sp-wdt.txt b/Documentation/devicetree/bindings/watchdog/zii,rave-sp-wdt.txt
>> new file mode 100644
>> index 000000000000..f242c3eed0dc
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/watchdog/zii,rave-sp-wdt.txt
>> @@ -0,0 +1,10 @@
>> +Zodiac Inflight Innovations RAVE Supervisory Processor Watchdog
>> +
>> +Required properties:
>> +
>> + - compatible: Should be "zii,rave-sp-watchdog"
>> +
>
> Must be a child of ? defined in ?

Good point, will add.

>
>> +Optional properties:
>> +
>> + - Two byte nvmem cell "wdt_timeout", specified as per
>> +   Documentation/devicetree/bindings/nvmem/nvmem.txt
>
> prop name: definition... is the normal format.
>
> Also, don't use '_' in property names.
>

OK, will fix.

Thanks,
Andrey Smirnov

Re: [PATCH v8 1/5] serdev: Make .remove in struct serdev_device_driver optional

[Sebastian Reichel]

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

Hi,

On Sat, Oct 21, 2017 at 10:08:40AM -0700, Guenter Roeck wrote:
> On 10/18/2017 10:01 AM, Andrey Smirnov wrote:
> > Using devres infrastructure it is possible to wirte a serdev driver
> 
> s/wirte/write/
> 
> otherwise
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

-- Sebastian

> > that doesn't have any code that needs to be called as a part of
> > .remove. Add code to make .remove optional.
> > 
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-serial@vger.kernel.org
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: cphealy@gmail.com
> > Cc: Guenter Roeck <linux@roeck-us.net>
> > 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>
> > Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> > Cc: Johan Hovold <johan@kernel.org>
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > ---
> >   drivers/tty/serdev/core.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> > index c68fb3a8ea1c..f500f6a2ca88 100644
> > --- a/drivers/tty/serdev/core.c
> > +++ b/drivers/tty/serdev/core.c
> > @@ -252,8 +252,8 @@ static int serdev_drv_probe(struct device *dev)
> >   static int serdev_drv_remove(struct device *dev)
> >   {
> >   	const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
> > -
> > -	sdrv->remove(to_serdev_device(dev));
> > +	if (sdrv->remove)
> > +		sdrv->remove(to_serdev_device(dev));
> >   	return 0;
> >   }
> > 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v8 2/5] serdev: Introduce devm_serdev_device_open()

[Sebastian Reichel]

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

Hi,

On Wed, Oct 18, 2017 at 10:01:33AM -0700, Andrey Smirnov wrote:
> Add code implementing managed version of serdev_device_open() for
> serdev device drivers that "open" the device during driver's lifecycle
> only once (e.g. opened in .probe() and closed in .remove()).
> 
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-serial@vger.kernel.org
> Cc: Rob Herring <robh@kernel.org>
> Cc: cphealy@gmail.com
> Cc: Guenter Roeck <linux@roeck-us.net>
> 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>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Johan Hovold <johan@kernel.org>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

-- Sebastian

>  Documentation/driver-model/devres.txt |  3 +++
>  drivers/tty/serdev/core.c             | 27 +++++++++++++++++++++++++++
>  include/linux/serdev.h                |  1 +
>  3 files changed, 31 insertions(+)
> 
> diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
> index 69f08c0f23a8..e9c6b5cfeec1 100644
> --- a/Documentation/driver-model/devres.txt
> +++ b/Documentation/driver-model/devres.txt
> @@ -383,6 +383,9 @@ RESET
>    devm_reset_control_get()
>    devm_reset_controller_register()
>  
> +SERDEV
> +  devm_serdev_device_open()
> +
>  SLAVE DMA ENGINE
>    devm_acpi_dma_controller_register()
>  
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index f500f6a2ca88..b3a785665c6f 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -116,6 +116,33 @@ void serdev_device_close(struct serdev_device *serdev)
>  }
>  EXPORT_SYMBOL_GPL(serdev_device_close);
>  
> +static void devm_serdev_device_release(struct device *dev, void *dr)
> +{
> +	serdev_device_close(*(struct serdev_device **)dr);
> +}
> +
> +int devm_serdev_device_open(struct device *dev, struct serdev_device *serdev)
> +{
> +	struct serdev_device **dr;
> +	int ret;
> +
> +	dr = devres_alloc(devm_serdev_device_release, sizeof(*dr), GFP_KERNEL);
> +	if (!dr)
> +		return -ENOMEM;
> +
> +	ret = serdev_device_open(serdev);
> +	if (ret) {
> +		devres_free(dr);
> +		return ret;
> +	}
> +
> +	*dr = serdev;
> +	devres_add(dev, dr);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(devm_serdev_device_open);
> +
>  void serdev_device_write_wakeup(struct serdev_device *serdev)
>  {
>  	complete(&serdev->write_comp);
> diff --git a/include/linux/serdev.h b/include/linux/serdev.h
> index e69402d4a8ae..9929063bd45d 100644
> --- a/include/linux/serdev.h
> +++ b/include/linux/serdev.h
> @@ -193,6 +193,7 @@ static inline int serdev_controller_receive_buf(struct serdev_controller *ctrl,
>  
>  int serdev_device_open(struct serdev_device *);
>  void serdev_device_close(struct serdev_device *);
> +int devm_serdev_device_open(struct device *, struct serdev_device *);
>  unsigned int serdev_device_set_baudrate(struct serdev_device *, unsigned int);
>  void serdev_device_set_flow_control(struct serdev_device *, bool);
>  int serdev_device_write_buf(struct serdev_device *, const unsigned char *, size_t);
> -- 
> 2.13.5
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]