[PATCH v11 0/8] Turris Omnia MCU driver

[PATCH v11 0/8] Turris Omnia MCU driver

[Marek Behún]

Hello Andy, Hans, Ilpo, Arnd, Gregory, and others,

this is v11 of the series adding Turris Omnia MCU driver.

Changes since v10:
- dropped patch 7 from v10 ("Add support for digital message signing via
  debugfs"). This must be done via different kernel API (should be
  doable via keyctl), but this requires more work which I currently
  don't have, unfortunately
- in patch 3 where I introduce support for MCU connected GPIOs
  changed u32 types to unsigned long where it made sense, in order
  to be able to use __assign_bit(), __set_bit(), test_bit().
  This was suggested by Andy.
- in patch 3 deduplicated code in omnia_gpio_get_multiple()
- moved the "fixing" in patch 3 of functions introduced in patch 2
  to patch 2, this was a rebasing error in v10
- changed date to September 2024 and KernelVersion to 6.11 in
  Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu

Links to previous cover letters (v1 to v10):
  https://patchwork.kernel.org/project/linux-soc/cover/20230823161012.6986-1-kabel@kernel.org/
  https://patchwork.kernel.org/project/linux-soc/cover/20230919103815.16818-1-kabel@kernel.org/
  https://patchwork.kernel.org/project/linux-soc/cover/20231023143130.11602-1-kabel@kernel.org/
  https://patchwork.kernel.org/project/linux-soc/cover/20231026161803.16750-1-kabel@kernel.org/
  https://patchwork.kernel.org/project/linux-soc/cover/20240323164359.21642-1-kabel@kernel.org/
  https://patchwork.kernel.org/project/linux-soc/cover/20240418121116.22184-1-kabel@kernel.org/
  https://patchwork.kernel.org/project/linux-soc/cover/20240424173809.7214-1-kabel@kernel.org/
  https://patchwork.kernel.org/project/linux-soc/cover/20240430115111.3453-1-kabel@kernel.org/
  https://patchwork.kernel.org/project/linux-soc/cover/20240508103118.23345-1-kabel@kernel.org/
  https://patchwork.kernel.org/project/linux-soc/cover/20240510101819.13551-1-kabel@kernel.org/

Marek Behún (8):
  dt-bindings: firmware: add cznic,turris-omnia-mcu binding
  platform: cznic: Add preliminary support for Turris Omnia MCU
  platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs
  platform: cznic: turris-omnia-mcu: Add support for poweroff and wakeup
  platform: cznic: turris-omnia-mcu: Add support for MCU watchdog
  platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG
  ARM: dts: turris-omnia: Add MCU system-controller node
  ARM: dts: turris-omnia: Add GPIO key node for front button

 .../sysfs-bus-i2c-devices-turris-omnia-mcu    |  113 ++
 .../firmware/cznic,turris-omnia-mcu.yaml      |   86 ++
 MAINTAINERS                                   |    4 +
 .../dts/marvell/armada-385-turris-omnia.dts   |   35 +-
 drivers/platform/Kconfig                      |    2 +
 drivers/platform/Makefile                     |    1 +
 drivers/platform/cznic/Kconfig                |   48 +
 drivers/platform/cznic/Makefile               |    8 +
 .../platform/cznic/turris-omnia-mcu-base.c    |  407 +++++++
 .../platform/cznic/turris-omnia-mcu-gpio.c    | 1071 +++++++++++++++++
 .../cznic/turris-omnia-mcu-sys-off-wakeup.c   |  257 ++++
 .../platform/cznic/turris-omnia-mcu-trng.c    |  103 ++
 .../cznic/turris-omnia-mcu-watchdog.c         |  128 ++
 drivers/platform/cznic/turris-omnia-mcu.h     |  194 +++
 include/linux/turris-omnia-mcu-interface.h    |  249 ++++
 15 files changed, 2705 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu
 create mode 100644 Documentation/devicetree/bindings/firmware/cznic,turris-omnia-mcu.yaml
 create mode 100644 drivers/platform/cznic/Kconfig
 create mode 100644 drivers/platform/cznic/Makefile
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-base.c
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-gpio.c
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-trng.c
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-watchdog.c
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu.h
 create mode 100644 include/linux/turris-omnia-mcu-interface.h

-- 
2.44.2

[PATCH v11 1/8] dt-bindings: firmware: add cznic,turris-omnia-mcu binding

[Marek Behún]

Add binding for cznic,turris-omnia-mcu, the device-tree node
representing the system-controller features provided by the MCU on the
Turris Omnia router.

Signed-off-by: Marek Behún <kabel@kernel.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../firmware/cznic,turris-omnia-mcu.yaml      | 86 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 87 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/cznic,turris-omnia-mcu.yaml

diff --git a/Documentation/devicetree/bindings/firmware/cznic,turris-omnia-mcu.yaml b/Documentation/devicetree/bindings/firmware/cznic,turris-omnia-mcu.yaml
new file mode 100644
index 000000000000..af9249695ef5
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/cznic,turris-omnia-mcu.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/cznic,turris-omnia-mcu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: CZ.NIC's Turris Omnia MCU
+
+maintainers:
+  - Marek Behún <kabel@kernel.org>
+
+description:
+  The MCU on Turris Omnia acts as a system controller providing additional
+  GPIOs, interrupts, watchdog, system power off and wakeup configuration.
+
+properties:
+  compatible:
+    const: cznic,turris-omnia-mcu
+
+  reg:
+    description: MCU I2C slave address
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  '#interrupt-cells':
+    const: 2
+    description: |
+      The first cell specifies the interrupt number (0 to 63), the second cell
+      specifies interrupt type (which can be one of IRQ_TYPE_EDGE_RISING,
+      IRQ_TYPE_EDGE_FALLING or IRQ_TYPE_EDGE_BOTH).
+      The interrupt numbers correspond sequentially to GPIO numbers, taking the
+      GPIO banks into account:
+        IRQ number   GPIO bank   GPIO pin within bank
+           0 - 15      0           0 - 15
+          16 - 47      1           0 - 31
+          48 - 63      2           0 - 15
+      There are several exceptions:
+        IRQ number   meaning
+          11           LED panel brightness changed by button press
+          13           TRNG entropy ready
+          14           ECDSA message signature computation done
+
+  gpio-controller: true
+
+  '#gpio-cells':
+    const: 3
+    description:
+      The first cell is bank number (0, 1 or 2), the second cell is pin number
+      within the bank (0 to 15 for banks 0 and 2, 0 to 31 for bank 1), and the
+      third cell specifies consumer flags.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-controller
+  - gpio-controller
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        system-controller@2a {
+            compatible = "cznic,turris-omnia-mcu";
+            reg = <0x2a>;
+
+            interrupt-parent = <&gpio1>;
+            interrupts = <11 IRQ_TYPE_NONE>;
+
+            gpio-controller;
+            #gpio-cells = <3>;
+
+            interrupt-controller;
+            #interrupt-cells = <2>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 8754ac2c259d..ee8b6dcd8f53 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2211,6 +2211,7 @@ F:	Documentation/ABI/testing/sysfs-bus-moxtet-devices
 F:	Documentation/ABI/testing/sysfs-firmware-turris-mox-rwtm
 F:	Documentation/devicetree/bindings/bus/moxtet.txt
 F:	Documentation/devicetree/bindings/firmware/cznic,turris-mox-rwtm.txt
+F:	Documentation/devicetree/bindings/firmware/cznic,turris-omnia-mcu.yaml
 F:	Documentation/devicetree/bindings/gpio/gpio-moxtet.txt
 F:	Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml
 F:	Documentation/devicetree/bindings/watchdog/armada-37xx-wdt.txt
-- 
2.44.2

[PATCH v11 2/8] platform: cznic: Add preliminary support for Turris Omnia MCU

[Marek Behún]

Add the basic skeleton for a new platform driver for the microcontroller
found on the Turris Omnia board.

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 .../sysfs-bus-i2c-devices-turris-omnia-mcu    |  81 ++++
 MAINTAINERS                                   |   3 +
 drivers/platform/Kconfig                      |   2 +
 drivers/platform/Makefile                     |   1 +
 drivers/platform/cznic/Kconfig                |  25 ++
 drivers/platform/cznic/Makefile               |   4 +
 .../platform/cznic/turris-omnia-mcu-base.c    | 393 ++++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |  74 ++++
 include/linux/turris-omnia-mcu-interface.h    | 249 +++++++++++
 9 files changed, 832 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu
 create mode 100644 drivers/platform/cznic/Kconfig
 create mode 100644 drivers/platform/cznic/Makefile
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-base.c
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu.h
 create mode 100644 include/linux/turris-omnia-mcu-interface.h

diff --git a/Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu b/Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu
new file mode 100644
index 000000000000..9bc5aad00de0
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu
@@ -0,0 +1,81 @@
+What:		/sys/bus/i2c/devices/<mcu_device>/board_revision
+Date:		September 2024
+KernelVersion:	6.11
+Contact:	Marek Behún <kabel@kernel.org>
+Description:	(RO) Contains board revision number.
+
+		Only available if board information is burned in the MCU (older
+		revisions have board information burned in the ATSHA204-A chip).
+
+		Format: %u.
+
+What:		/sys/bus/i2c/devices/<mcu_device>/first_mac_address
+Date:		September 2024
+KernelVersion:	6.11
+Contact:	Marek Behún <kabel@kernel.org>
+Description:	(RO) Contains device first MAC address. Each Turris Omnia is
+		allocated 3 MAC addresses. The two additional addresses are
+		computed from the first one by incrementing it.
+
+		Only available if board information is burned in the MCU (older
+		revisions have board information burned in the ATSHA204-A chip).
+
+		Format: %pM.
+
+What:		/sys/bus/i2c/devices/<mcu_device>/fw_features
+Date:		September 2024
+KernelVersion:	6.11
+Contact:	Marek Behún <kabel@kernel.org>
+Description:	(RO) Newer versions of the microcontroller firmware report the
+		features they support. These can be read from this file. If the
+		MCU firmware is too old, this file reads 0x0.
+
+		Format: 0x%x.
+
+What:		/sys/bus/i2c/devices/<mcu_device>/fw_version_hash_application
+Date:		September 2024
+KernelVersion:	6.11
+Contact:	Marek Behún <kabel@kernel.org>
+Description:	(RO) Contains the version hash (commit hash) of the application
+		part of the microcontroller firmware.
+
+		Format: %s.
+
+What:		/sys/bus/i2c/devices/<mcu_device>/fw_version_hash_bootloader
+Date:		September 2024
+KernelVersion:	6.11
+Contact:	Marek Behún <kabel@kernel.org>
+Description:	(RO) Contains the version hash (commit hash) of the bootloader
+		part of the microcontroller firmware.
+
+		Format: %s.
+
+What:		/sys/bus/i2c/devices/<mcu_device>/mcu_type
+Date:		September 2024
+KernelVersion:	6.11
+Contact:	Marek Behún <kabel@kernel.org>
+Description:	(RO) Contains the microcontroller type (STM32, GD32, MKL).
+
+		Format: %s.
+
+What:		/sys/bus/i2c/devices/<mcu_device>/reset_selector
+Date:		September 2024
+KernelVersion:	6.11
+Contact:	Marek Behún <kabel@kernel.org>
+Description:	(RO) Contains the selected factory reset level, determined by
+		how long the rear reset button was held by the user during board
+		reset.
+
+		Format: %i.
+
+What:		/sys/bus/i2c/devices/<mcu_device>/serial_number
+Date:		September 2024
+KernelVersion:	6.11
+Contact:	Marek Behún <kabel@kernel.org>
+Description:	(RO) Contains the 64-bit board serial number in hexadecimal
+		format.
+
+		Only available if board information is burned in the MCU (older
+		revisions have board information burned in the ATSHA204-A chip).
+
+		Format: %016X.
diff --git a/MAINTAINERS b/MAINTAINERS
index ee8b6dcd8f53..9f6dca4627c2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2207,6 +2207,7 @@ M:	Marek Behún <kabel@kernel.org>
 S:	Maintained
 W:	https://www.turris.cz/
 F:	Documentation/ABI/testing/debugfs-moxtet
+F:	Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu
 F:	Documentation/ABI/testing/sysfs-bus-moxtet-devices
 F:	Documentation/ABI/testing/sysfs-firmware-turris-mox-rwtm
 F:	Documentation/devicetree/bindings/bus/moxtet.txt
@@ -2220,10 +2221,12 @@ F:	drivers/firmware/turris-mox-rwtm.c
 F:	drivers/gpio/gpio-moxtet.c
 F:	drivers/leds/leds-turris-omnia.c
 F:	drivers/mailbox/armada-37xx-rwtm-mailbox.c
+F:	drivers/platform/cznic/
 F:	drivers/watchdog/armada_37xx_wdt.c
 F:	include/dt-bindings/bus/moxtet.h
 F:	include/linux/armada-37xx-rwtm-mailbox.h
 F:	include/linux/moxtet.h
+F:	include/linux/turris-omnia-mcu-interface.h
 
 ARM/FARADAY FA526 PORT
 M:	Hans Ulli Kroll <ulli.kroll@googlemail.com>
diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
index 81a298517df2..960fd6a82450 100644
--- a/drivers/platform/Kconfig
+++ b/drivers/platform/Kconfig
@@ -7,6 +7,8 @@ source "drivers/platform/goldfish/Kconfig"
 
 source "drivers/platform/chrome/Kconfig"
 
+source "drivers/platform/cznic/Kconfig"
+
 source "drivers/platform/mellanox/Kconfig"
 
 source "drivers/platform/olpc/Kconfig"
diff --git a/drivers/platform/Makefile b/drivers/platform/Makefile
index fbbe4f77aa5d..bf69cc8d7429 100644
--- a/drivers/platform/Makefile
+++ b/drivers/platform/Makefile
@@ -10,5 +10,6 @@ obj-$(CONFIG_MIPS)		+= mips/
 obj-$(CONFIG_OLPC_EC)		+= olpc/
 obj-$(CONFIG_GOLDFISH)		+= goldfish/
 obj-$(CONFIG_CHROME_PLATFORMS)	+= chrome/
+obj-$(CONFIG_CZNIC_PLATFORMS)	+= cznic/
 obj-$(CONFIG_SURFACE_PLATFORMS)	+= surface/
 obj-$(CONFIG_ARM64)		+= arm64/
diff --git a/drivers/platform/cznic/Kconfig b/drivers/platform/cznic/Kconfig
new file mode 100644
index 000000000000..db5f4a673d28
--- /dev/null
+++ b/drivers/platform/cznic/Kconfig
@@ -0,0 +1,25 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# For a description of the syntax of this configuration file,
+# see Documentation/kbuild/kconfig-language.rst.
+#
+
+menuconfig CZNIC_PLATFORMS
+	bool "Platform support for CZ.NIC's Turris hardware"
+	help
+	  Say Y here to be able to choose driver support for CZ.NIC's Turris
+	  devices. This option alone does not add any kernel code.
+
+if CZNIC_PLATFORMS
+
+config TURRIS_OMNIA_MCU
+	tristate "Turris Omnia MCU driver"
+	depends on MACH_ARMADA_38X || COMPILE_TEST
+	depends on I2C
+	help
+	  Say Y here to add support for the features implemented by the
+	  microcontroller on the CZ.NIC's Turris Omnia SOHO router.
+	  To compile this driver as a module, choose M here; the module will be
+	  called turris-omnia-mcu.
+
+endif # CZNIC_PLATFORMS
diff --git a/drivers/platform/cznic/Makefile b/drivers/platform/cznic/Makefile
new file mode 100644
index 000000000000..31adca73bb94
--- /dev/null
+++ b/drivers/platform/cznic/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+obj-$(CONFIG_TURRIS_OMNIA_MCU)	+= turris-omnia-mcu.o
+turris-omnia-mcu-y		:= turris-omnia-mcu-base.o
diff --git a/drivers/platform/cznic/turris-omnia-mcu-base.c b/drivers/platform/cznic/turris-omnia-mcu-base.c
new file mode 100644
index 000000000000..a5ae2ba9abb6
--- /dev/null
+++ b/drivers/platform/cznic/turris-omnia-mcu-base.c
@@ -0,0 +1,393 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CZ.NIC's Turris Omnia MCU driver
+ *
+ * 2024 by Marek Behún <kabel@kernel.org>
+ */
+
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/device.h>
+#include <linux/hex.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/turris-omnia-mcu-interface.h>
+#include <linux/types.h>
+#include <linux/string.h>
+#include <linux/sysfs.h>
+
+#include "turris-omnia-mcu.h"
+
+#define OMNIA_FW_VERSION_LEN		20
+#define OMNIA_FW_VERSION_HEX_LEN	(2 * OMNIA_FW_VERSION_LEN + 1)
+#define OMNIA_BOARD_INFO_LEN		16
+
+int omnia_cmd_write_read(const struct i2c_client *client,
+			 void *cmd, unsigned int cmd_len,
+			 void *reply, unsigned int reply_len)
+{
+	struct i2c_msg msgs[2];
+	int ret, num;
+
+	msgs[0].addr = client->addr;
+	msgs[0].flags = 0;
+	msgs[0].len = cmd_len;
+	msgs[0].buf = cmd;
+	num = 1;
+
+	if (reply_len) {
+		msgs[1].addr = client->addr;
+		msgs[1].flags = I2C_M_RD;
+		msgs[1].len = reply_len;
+		msgs[1].buf = reply;
+		num++;
+	}
+
+	ret = i2c_transfer(client->adapter, msgs, num);
+	if (ret < 0)
+		return ret;
+	if (ret != num)
+		return -EIO;
+
+	return 0;
+}
+
+static int omnia_get_version_hash(struct omnia_mcu *mcu, bool bootloader,
+				  char version[static OMNIA_FW_VERSION_HEX_LEN])
+{
+	u8 reply[OMNIA_FW_VERSION_LEN];
+	char *p;
+	int err;
+
+	err = omnia_cmd_read(mcu->client,
+			     bootloader ? OMNIA_CMD_GET_FW_VERSION_BOOT
+					: OMNIA_CMD_GET_FW_VERSION_APP,
+			     reply, sizeof(reply));
+	if (err)
+		return err;
+
+	p = bin2hex(version, reply, OMNIA_FW_VERSION_LEN);
+	*p = '\0';
+
+	return 0;
+}
+
+static ssize_t fw_version_hash_show(struct device *dev, char *buf,
+				    bool bootloader)
+{
+	struct omnia_mcu *mcu = dev_get_drvdata(dev);
+	char version[OMNIA_FW_VERSION_HEX_LEN];
+	int err;
+
+	err = omnia_get_version_hash(mcu, bootloader, version);
+	if (err)
+		return err;
+
+	return sysfs_emit(buf, "%s\n", version);
+}
+
+static ssize_t fw_version_hash_application_show(struct device *dev,
+						struct device_attribute *a,
+						char *buf)
+{
+	return fw_version_hash_show(dev, buf, false);
+}
+static DEVICE_ATTR_RO(fw_version_hash_application);
+
+static ssize_t fw_version_hash_bootloader_show(struct device *dev,
+					       struct device_attribute *a,
+					       char *buf)
+{
+	return fw_version_hash_show(dev, buf, true);
+}
+static DEVICE_ATTR_RO(fw_version_hash_bootloader);
+
+static ssize_t fw_features_show(struct device *dev, struct device_attribute *a,
+				char *buf)
+{
+	struct omnia_mcu *mcu = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "0x%x\n", mcu->features);
+}
+static DEVICE_ATTR_RO(fw_features);
+
+static ssize_t mcu_type_show(struct device *dev, struct device_attribute *a,
+			     char *buf)
+{
+	struct omnia_mcu *mcu = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%s\n", mcu->type);
+}
+static DEVICE_ATTR_RO(mcu_type);
+
+static ssize_t reset_selector_show(struct device *dev,
+				   struct device_attribute *a, char *buf)
+{
+	u8 reply;
+	int err;
+
+	err = omnia_cmd_read_u8(to_i2c_client(dev), OMNIA_CMD_GET_RESET,
+				&reply);
+	if (err)
+		return err;
+
+	return sysfs_emit(buf, "%d\n", reply);
+}
+static DEVICE_ATTR_RO(reset_selector);
+
+static ssize_t serial_number_show(struct device *dev,
+				  struct device_attribute *a, char *buf)
+{
+	struct omnia_mcu *mcu = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%016llX\n", mcu->board_serial_number);
+}
+static DEVICE_ATTR_RO(serial_number);
+
+static ssize_t first_mac_address_show(struct device *dev,
+				      struct device_attribute *a, char *buf)
+{
+	struct omnia_mcu *mcu = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%pM\n", mcu->board_first_mac);
+}
+static DEVICE_ATTR_RO(first_mac_address);
+
+static ssize_t board_revision_show(struct device *dev,
+				   struct device_attribute *a, char *buf)
+{
+	struct omnia_mcu *mcu = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%u\n", mcu->board_revision);
+}
+static DEVICE_ATTR_RO(board_revision);
+
+static struct attribute *omnia_mcu_base_attrs[] = {
+	&dev_attr_fw_version_hash_application.attr,
+	&dev_attr_fw_version_hash_bootloader.attr,
+	&dev_attr_fw_features.attr,
+	&dev_attr_mcu_type.attr,
+	&dev_attr_reset_selector.attr,
+	&dev_attr_serial_number.attr,
+	&dev_attr_first_mac_address.attr,
+	&dev_attr_board_revision.attr,
+	NULL
+};
+
+static umode_t omnia_mcu_base_attrs_visible(struct kobject *kobj,
+					    struct attribute *a, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct omnia_mcu *mcu = dev_get_drvdata(dev);
+
+	if ((a == &dev_attr_serial_number.attr ||
+	     a == &dev_attr_first_mac_address.attr ||
+	     a == &dev_attr_board_revision.attr) &&
+	    !(mcu->features & OMNIA_FEAT_BOARD_INFO))
+		return 0;
+
+	return a->mode;
+}
+
+static const struct attribute_group omnia_mcu_base_group = {
+	.attrs = omnia_mcu_base_attrs,
+	.is_visible = omnia_mcu_base_attrs_visible,
+};
+
+static const struct attribute_group *omnia_mcu_groups[] = {
+	&omnia_mcu_base_group,
+	NULL
+};
+
+static void omnia_mcu_print_version_hash(struct omnia_mcu *mcu, bool bootloader)
+{
+	const char *type = bootloader ? "bootloader" : "application";
+	struct device *dev = &mcu->client->dev;
+	char version[OMNIA_FW_VERSION_HEX_LEN];
+	int err;
+
+	err = omnia_get_version_hash(mcu, bootloader, version);
+	if (err) {
+		dev_err(dev, "Cannot read MCU %s firmware version: %d\n",
+			type, err);
+		return;
+	}
+
+	dev_info(dev, "MCU %s firmware version hash: %s\n", type, version);
+}
+
+static const char *omnia_status_to_mcu_type(u16 status)
+{
+	switch (status & OMNIA_STS_MCU_TYPE_MASK) {
+	case OMNIA_STS_MCU_TYPE_STM32:
+		return "STM32";
+	case OMNIA_STS_MCU_TYPE_GD32:
+		return "GD32";
+	case OMNIA_STS_MCU_TYPE_MKL:
+		return "MKL";
+	default:
+		return "unknown";
+	}
+}
+
+static void omnia_info_missing_feature(struct device *dev, const char *feature)
+{
+	dev_info(dev,
+		 "Your board's MCU firmware does not support the %s feature.\n",
+		 feature);
+}
+
+static int omnia_mcu_read_features(struct omnia_mcu *mcu)
+{
+	static const struct {
+		u16 mask;
+		const char *name;
+	} features[] = {
+#define _DEF_FEAT(_n, _m) { OMNIA_FEAT_ ## _n, _m }
+		_DEF_FEAT(EXT_CMDS,		"extended control and status"),
+		_DEF_FEAT(WDT_PING,		"watchdog pinging"),
+		_DEF_FEAT(LED_STATE_EXT_MASK,	"peripheral LED pins reading"),
+		_DEF_FEAT(NEW_INT_API,		"new interrupt API"),
+		_DEF_FEAT(POWEROFF_WAKEUP,	"poweroff and wakeup"),
+		_DEF_FEAT(TRNG,			"true random number generator"),
+#undef _DEF_FEAT
+	};
+	struct i2c_client *client = mcu->client;
+	struct device *dev = &client->dev;
+	bool suggest_fw_upgrade = false;
+	u16 status;
+	int err;
+
+	/* status word holds MCU type, which we need below */
+	err = omnia_cmd_read_u16(client, OMNIA_CMD_GET_STATUS_WORD, &status);
+	if (err)
+		return err;
+
+	/*
+	 * Check whether MCU firmware supports the OMNIA_CMD_GET_FEATURES
+	 * command.
+	 */
+	if (status & OMNIA_STS_FEATURES_SUPPORTED) {
+		/* try read 32-bit features */
+		err = omnia_cmd_read_u32(client, OMNIA_CMD_GET_FEATURES,
+					 &mcu->features);
+		if (err) {
+			/* try read 16-bit features */
+			u16 features16;
+
+			err = omnia_cmd_read_u16(client, OMNIA_CMD_GET_FEATURES,
+						 &features16);
+			if (err)
+				return err;
+
+			mcu->features = features16;
+		} else {
+			if (mcu->features & OMNIA_FEAT_FROM_BIT_16_INVALID)
+				mcu->features &= GENMASK(15, 0);
+		}
+	} else {
+		dev_info(dev,
+			 "Your board's MCU firmware does not support feature reading.\n");
+		suggest_fw_upgrade = true;
+	}
+
+	mcu->type = omnia_status_to_mcu_type(status);
+	dev_info(dev, "MCU type %s%s\n", mcu->type,
+		 (mcu->features & OMNIA_FEAT_PERIPH_MCU) ?
+			", with peripheral resets wired" : "");
+
+	omnia_mcu_print_version_hash(mcu, true);
+
+	if (mcu->features & OMNIA_FEAT_BOOTLOADER)
+		dev_warn(dev,
+			 "MCU is running bootloader firmware. Was firmware upgrade interrupted?\n");
+	else
+		omnia_mcu_print_version_hash(mcu, false);
+
+	for (unsigned int i = 0; i < ARRAY_SIZE(features); i++) {
+		if (mcu->features & features[i].mask)
+			continue;
+
+		omnia_info_missing_feature(dev, features[i].name);
+		suggest_fw_upgrade = true;
+	}
+
+	if (suggest_fw_upgrade)
+		dev_info(dev,
+			 "Consider upgrading MCU firmware with the omnia-mcutool utility.\n");
+
+	return 0;
+}
+
+static int omnia_mcu_read_board_info(struct omnia_mcu *mcu)
+{
+	u8 reply[1 + OMNIA_BOARD_INFO_LEN];
+	int err;
+
+	err = omnia_cmd_read(mcu->client, OMNIA_CMD_BOARD_INFO_GET, reply,
+			     sizeof(reply));
+	if (err)
+		return err;
+
+	if (reply[0] != OMNIA_BOARD_INFO_LEN)
+		return -EIO;
+
+	mcu->board_serial_number = get_unaligned_le64(&reply[1]);
+
+	/* we can't use ether_addr_copy() because reply is not u16-aligned */
+	memcpy(mcu->board_first_mac, &reply[9], ETH_ALEN);
+
+	mcu->board_revision = reply[15];
+
+	return 0;
+}
+
+static int omnia_mcu_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct omnia_mcu *mcu;
+	int err;
+
+	if (!client->irq)
+		return dev_err_probe(dev, -EINVAL, "IRQ resource not found\n");
+
+	mcu = devm_kzalloc(dev, sizeof(*mcu), GFP_KERNEL);
+	if (!mcu)
+		return -ENOMEM;
+
+	mcu->client = client;
+	i2c_set_clientdata(client, mcu);
+
+	err = omnia_mcu_read_features(mcu);
+	if (err)
+		return dev_err_probe(dev, err,
+				     "Cannot determine MCU supported features\n");
+
+	if (mcu->features & OMNIA_FEAT_BOARD_INFO) {
+		err = omnia_mcu_read_board_info(mcu);
+		if (err)
+			return dev_err_probe(dev, err,
+					     "Cannot read board info\n");
+	}
+
+	return 0;
+}
+
+static const struct of_device_id of_omnia_mcu_match[] = {
+	{ .compatible = "cznic,turris-omnia-mcu" },
+	{}
+};
+
+static struct i2c_driver omnia_mcu_driver = {
+	.probe		= omnia_mcu_probe,
+	.driver		= {
+		.name	= "turris-omnia-mcu",
+		.of_match_table = of_omnia_mcu_match,
+		.dev_groups = omnia_mcu_groups,
+	},
+};
+module_i2c_driver(omnia_mcu_driver);
+
+MODULE_AUTHOR("Marek Behun <kabel@kernel.org>");
+MODULE_DESCRIPTION("CZ.NIC's Turris Omnia MCU");
+MODULE_LICENSE("GPL");
diff --git a/drivers/platform/cznic/turris-omnia-mcu.h b/drivers/platform/cznic/turris-omnia-mcu.h
new file mode 100644
index 000000000000..3d0daa6f13ef
--- /dev/null
+++ b/drivers/platform/cznic/turris-omnia-mcu.h
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * CZ.NIC's Turris Omnia MCU driver
+ *
+ * 2024 by Marek Behún <kabel@kernel.org>
+ */
+
+#ifndef __TURRIS_OMNIA_MCU_H
+#define __TURRIS_OMNIA_MCU_H
+
+#include <linux/if_ether.h>
+#include <linux/types.h>
+#include <asm/byteorder.h>
+
+struct i2c_client;
+
+struct omnia_mcu {
+	struct i2c_client *client;
+	const char *type;
+	u32 features;
+
+	/* board information */
+	u64 board_serial_number;
+	u8 board_first_mac[ETH_ALEN];
+	u8 board_revision;
+};
+
+int omnia_cmd_write_read(const struct i2c_client *client,
+			 void *cmd, unsigned int cmd_len,
+			 void *reply, unsigned int reply_len);
+
+static inline int omnia_cmd_read(const struct i2c_client *client, u8 cmd,
+				 void *reply, unsigned int len)
+{
+	return omnia_cmd_write_read(client, &cmd, 1, reply, len);
+}
+
+static inline int omnia_cmd_read_u32(const struct i2c_client *client, u8 cmd,
+				     u32 *dst)
+{
+	__le32 reply;
+	int err;
+
+	err = omnia_cmd_read(client, cmd, &reply, sizeof(reply));
+	if (err)
+		return err;
+
+	*dst = le32_to_cpu(reply);
+
+	return 0;
+}
+
+static inline int omnia_cmd_read_u16(const struct i2c_client *client, u8 cmd,
+				     u16 *dst)
+{
+	__le16 reply;
+	int err;
+
+	err = omnia_cmd_read(client, cmd, &reply, sizeof(reply));
+	if (err)
+		return err;
+
+	*dst = le16_to_cpu(reply);
+
+	return 0;
+}
+
+static inline int omnia_cmd_read_u8(const struct i2c_client *client, u8 cmd,
+				    u8 *reply)
+{
+	return omnia_cmd_read(client, cmd, reply, sizeof(*reply));
+}
+
+#endif /* __TURRIS_OMNIA_MCU_H */
diff --git a/include/linux/turris-omnia-mcu-interface.h b/include/linux/turris-omnia-mcu-interface.h
new file mode 100644
index 000000000000..2da8cbeb158a
--- /dev/null
+++ b/include/linux/turris-omnia-mcu-interface.h
@@ -0,0 +1,249 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * CZ.NIC's Turris Omnia MCU I2C interface commands definitions
+ *
+ * 2024 by Marek Behún <kabel@kernel.org>
+ */
+
+#ifndef __TURRIS_OMNIA_MCU_INTERFACE_H
+#define __TURRIS_OMNIA_MCU_INTERFACE_H
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+
+enum omnia_commands_e {
+	OMNIA_CMD_GET_STATUS_WORD		= 0x01, /* slave sends status word back */
+	OMNIA_CMD_GENERAL_CONTROL		= 0x02,
+	OMNIA_CMD_LED_MODE			= 0x03, /* default/user */
+	OMNIA_CMD_LED_STATE			= 0x04, /* LED on/off */
+	OMNIA_CMD_LED_COLOR			= 0x05, /* LED number + RED + GREEN + BLUE */
+	OMNIA_CMD_USER_VOLTAGE			= 0x06,
+	OMNIA_CMD_SET_BRIGHTNESS		= 0x07,
+	OMNIA_CMD_GET_BRIGHTNESS		= 0x08,
+	OMNIA_CMD_GET_RESET			= 0x09,
+	OMNIA_CMD_GET_FW_VERSION_APP		= 0x0A, /* 20B git hash number */
+	OMNIA_CMD_SET_WATCHDOG_STATE		= 0x0B, /* 0 - disable
+							 * 1 - enable / ping
+							 * after boot watchdog is started
+							 * with 2 minutes timeout
+							 */
+
+	/* OMNIA_CMD_WATCHDOG_STATUS		= 0x0C, not implemented anymore */
+
+	OMNIA_CMD_GET_WATCHDOG_STATE		= 0x0D,
+	OMNIA_CMD_GET_FW_VERSION_BOOT		= 0x0E, /* 20B Git hash number */
+	OMNIA_CMD_GET_FW_CHECKSUM		= 0x0F, /* 4B length, 4B checksum */
+
+	/* available if FEATURES_SUPPORTED bit set in status word */
+	OMNIA_CMD_GET_FEATURES			= 0x10,
+
+	/* available if EXT_CMD bit set in features */
+	OMNIA_CMD_GET_EXT_STATUS_DWORD		= 0x11,
+	OMNIA_CMD_EXT_CONTROL			= 0x12,
+	OMNIA_CMD_GET_EXT_CONTROL_STATUS	= 0x13,
+
+	/* available if NEW_INT_API bit set in features */
+	OMNIA_CMD_GET_INT_AND_CLEAR		= 0x14,
+	OMNIA_CMD_GET_INT_MASK			= 0x15,
+	OMNIA_CMD_SET_INT_MASK			= 0x16,
+
+	/* available if FLASHING bit set in features */
+	OMNIA_CMD_FLASH				= 0x19,
+
+	/* available if WDT_PING bit set in features */
+	OMNIA_CMD_SET_WDT_TIMEOUT		= 0x20,
+	OMNIA_CMD_GET_WDT_TIMELEFT		= 0x21,
+
+	/* available if POWEROFF_WAKEUP bit set in features */
+	OMNIA_CMD_SET_WAKEUP			= 0x22,
+	OMNIA_CMD_GET_UPTIME_AND_WAKEUP		= 0x23,
+	OMNIA_CMD_POWER_OFF			= 0x24,
+
+	/* available if USB_OVC_PROT_SETTING bit set in features */
+	OMNIA_CMD_SET_USB_OVC_PROT		= 0x25,
+	OMNIA_CMD_GET_USB_OVC_PROT		= 0x26,
+
+	/* available if TRNG bit set in features */
+	OMNIA_CMD_TRNG_COLLECT_ENTROPY		= 0x28,
+
+	/* available if CRYPTO bit set in features */
+	OMNIA_CMD_CRYPTO_GET_PUBLIC_KEY		= 0x29,
+	OMNIA_CMD_CRYPTO_SIGN_MESSAGE		= 0x2A,
+	OMNIA_CMD_CRYPTO_COLLECT_SIGNATURE	= 0x2B,
+
+	/* available if BOARD_INFO it set in features */
+	OMNIA_CMD_BOARD_INFO_GET		= 0x2C,
+	OMNIA_CMD_BOARD_INFO_BURN		= 0x2D,
+
+	/* available only at address 0x2b (LED-controller) */
+	/* available only if LED_GAMMA_CORRECTION bit set in features */
+	OMNIA_CMD_SET_GAMMA_CORRECTION		= 0x30,
+	OMNIA_CMD_GET_GAMMA_CORRECTION		= 0x31,
+
+	/* available only at address 0x2b (LED-controller) */
+	/* available only if PER_LED_CORRECTION bit set in features */
+	/* available only if FROM_BIT_16_INVALID bit NOT set in features */
+	OMNIA_CMD_SET_LED_CORRECTIONS		= 0x32,
+	OMNIA_CMD_GET_LED_CORRECTIONS		= 0x33,
+};
+
+enum omnia_flashing_commands_e {
+	OMNIA_FLASH_CMD_UNLOCK		= 0x01,
+	OMNIA_FLASH_CMD_SIZE_AND_CSUM	= 0x02,
+	OMNIA_FLASH_CMD_PROGRAM		= 0x03,
+	OMNIA_FLASH_CMD_RESET		= 0x04,
+};
+
+enum omnia_sts_word_e {
+	OMNIA_STS_MCU_TYPE_MASK			= GENMASK(1, 0),
+	OMNIA_STS_MCU_TYPE_STM32		= FIELD_PREP_CONST(OMNIA_STS_MCU_TYPE_MASK, 0),
+	OMNIA_STS_MCU_TYPE_GD32			= FIELD_PREP_CONST(OMNIA_STS_MCU_TYPE_MASK, 1),
+	OMNIA_STS_MCU_TYPE_MKL			= FIELD_PREP_CONST(OMNIA_STS_MCU_TYPE_MASK, 2),
+	OMNIA_STS_FEATURES_SUPPORTED		= BIT(2),
+	OMNIA_STS_USER_REGULATOR_NOT_SUPPORTED	= BIT(3),
+	OMNIA_STS_CARD_DET			= BIT(4),
+	OMNIA_STS_MSATA_IND			= BIT(5),
+	OMNIA_STS_USB30_OVC			= BIT(6),
+	OMNIA_STS_USB31_OVC			= BIT(7),
+	OMNIA_STS_USB30_PWRON			= BIT(8),
+	OMNIA_STS_USB31_PWRON			= BIT(9),
+	OMNIA_STS_ENABLE_4V5			= BIT(10),
+	OMNIA_STS_BUTTON_MODE			= BIT(11),
+	OMNIA_STS_BUTTON_PRESSED		= BIT(12),
+	OMNIA_STS_BUTTON_COUNTER_MASK		= GENMASK(15, 13),
+};
+
+enum omnia_ctl_byte_e {
+	OMNIA_CTL_LIGHT_RST	= BIT(0),
+	OMNIA_CTL_HARD_RST	= BIT(1),
+	/* BIT(2) is currently reserved */
+	OMNIA_CTL_USB30_PWRON	= BIT(3),
+	OMNIA_CTL_USB31_PWRON	= BIT(4),
+	OMNIA_CTL_ENABLE_4V5	= BIT(5),
+	OMNIA_CTL_BUTTON_MODE	= BIT(6),
+	OMNIA_CTL_BOOTLOADER	= BIT(7),
+};
+
+enum omnia_features_e {
+	OMNIA_FEAT_PERIPH_MCU		= BIT(0),
+	OMNIA_FEAT_EXT_CMDS		= BIT(1),
+	OMNIA_FEAT_WDT_PING		= BIT(2),
+	OMNIA_FEAT_LED_STATE_EXT_MASK	= GENMASK(4, 3),
+	OMNIA_FEAT_LED_STATE_EXT	= FIELD_PREP_CONST(OMNIA_FEAT_LED_STATE_EXT_MASK, 1),
+	OMNIA_FEAT_LED_STATE_EXT_V32	= FIELD_PREP_CONST(OMNIA_FEAT_LED_STATE_EXT_MASK, 2),
+	OMNIA_FEAT_LED_GAMMA_CORRECTION	= BIT(5),
+	OMNIA_FEAT_NEW_INT_API		= BIT(6),
+	OMNIA_FEAT_BOOTLOADER		= BIT(7),
+	OMNIA_FEAT_FLASHING		= BIT(8),
+	OMNIA_FEAT_NEW_MESSAGE_API	= BIT(9),
+	OMNIA_FEAT_BRIGHTNESS_INT	= BIT(10),
+	OMNIA_FEAT_POWEROFF_WAKEUP	= BIT(11),
+	OMNIA_FEAT_CAN_OLD_MESSAGE_API	= BIT(12),
+	OMNIA_FEAT_TRNG			= BIT(13),
+	OMNIA_FEAT_CRYPTO		= BIT(14),
+	OMNIA_FEAT_BOARD_INFO		= BIT(15),
+
+	/*
+	 * Orginally the features command replied only 16 bits. If more were
+	 * read, either the I2C transaction failed or 0xff bytes were sent.
+	 * Therefore to consider bits 16 - 31 valid, one bit (20) was reserved
+	 * to be zero.
+	 */
+
+	/* Bits 16 - 19 correspond to bits 0 - 3 of status word */
+	OMNIA_FEAT_MCU_TYPE_MASK		= GENMASK(17, 16),
+	OMNIA_FEAT_MCU_TYPE_STM32		= FIELD_PREP_CONST(OMNIA_FEAT_MCU_TYPE_MASK, 0),
+	OMNIA_FEAT_MCU_TYPE_GD32		= FIELD_PREP_CONST(OMNIA_FEAT_MCU_TYPE_MASK, 1),
+	OMNIA_FEAT_MCU_TYPE_MKL			= FIELD_PREP_CONST(OMNIA_FEAT_MCU_TYPE_MASK, 2),
+	OMNIA_FEAT_FEATURES_SUPPORTED		= BIT(18),
+	OMNIA_FEAT_USER_REGULATOR_NOT_SUPPORTED	= BIT(19),
+
+	/* must not be set */
+	OMNIA_FEAT_FROM_BIT_16_INVALID	= BIT(20),
+
+	OMNIA_FEAT_PER_LED_CORRECTION	= BIT(21),
+	OMNIA_FEAT_USB_OVC_PROT_SETTING	= BIT(22),
+};
+
+enum omnia_ext_sts_dword_e {
+	OMNIA_EXT_STS_SFP_nDET		= BIT(0),
+	OMNIA_EXT_STS_LED_STATES_MASK	= GENMASK(31, 12),
+	OMNIA_EXT_STS_WLAN0_MSATA_LED	= BIT(12),
+	OMNIA_EXT_STS_WLAN1_LED		= BIT(13),
+	OMNIA_EXT_STS_WLAN2_LED		= BIT(14),
+	OMNIA_EXT_STS_WPAN0_LED		= BIT(15),
+	OMNIA_EXT_STS_WPAN1_LED		= BIT(16),
+	OMNIA_EXT_STS_WPAN2_LED		= BIT(17),
+	OMNIA_EXT_STS_WAN_LED0		= BIT(18),
+	OMNIA_EXT_STS_WAN_LED1		= BIT(19),
+	OMNIA_EXT_STS_LAN0_LED0		= BIT(20),
+	OMNIA_EXT_STS_LAN0_LED1		= BIT(21),
+	OMNIA_EXT_STS_LAN1_LED0		= BIT(22),
+	OMNIA_EXT_STS_LAN1_LED1		= BIT(23),
+	OMNIA_EXT_STS_LAN2_LED0		= BIT(24),
+	OMNIA_EXT_STS_LAN2_LED1		= BIT(25),
+	OMNIA_EXT_STS_LAN3_LED0		= BIT(26),
+	OMNIA_EXT_STS_LAN3_LED1		= BIT(27),
+	OMNIA_EXT_STS_LAN4_LED0		= BIT(28),
+	OMNIA_EXT_STS_LAN4_LED1		= BIT(29),
+	OMNIA_EXT_STS_LAN5_LED0		= BIT(30),
+	OMNIA_EXT_STS_LAN5_LED1		= BIT(31),
+};
+
+enum omnia_ext_ctl_e {
+	OMNIA_EXT_CTL_nRES_MMC		= BIT(0),
+	OMNIA_EXT_CTL_nRES_LAN		= BIT(1),
+	OMNIA_EXT_CTL_nRES_PHY		= BIT(2),
+	OMNIA_EXT_CTL_nPERST0		= BIT(3),
+	OMNIA_EXT_CTL_nPERST1		= BIT(4),
+	OMNIA_EXT_CTL_nPERST2		= BIT(5),
+	OMNIA_EXT_CTL_PHY_SFP		= BIT(6),
+	OMNIA_EXT_CTL_PHY_SFP_AUTO	= BIT(7),
+	OMNIA_EXT_CTL_nVHV_CTRL		= BIT(8),
+};
+
+enum omnia_int_e {
+	OMNIA_INT_CARD_DET		= BIT(0),
+	OMNIA_INT_MSATA_IND		= BIT(1),
+	OMNIA_INT_USB30_OVC		= BIT(2),
+	OMNIA_INT_USB31_OVC		= BIT(3),
+	OMNIA_INT_BUTTON_PRESSED	= BIT(4),
+	OMNIA_INT_SFP_nDET		= BIT(5),
+	OMNIA_INT_BRIGHTNESS_CHANGED	= BIT(6),
+	OMNIA_INT_TRNG			= BIT(7),
+	OMNIA_INT_MESSAGE_SIGNED	= BIT(8),
+
+	OMNIA_INT_LED_STATES_MASK	= GENMASK(31, 12),
+	OMNIA_INT_WLAN0_MSATA_LED	= BIT(12),
+	OMNIA_INT_WLAN1_LED		= BIT(13),
+	OMNIA_INT_WLAN2_LED		= BIT(14),
+	OMNIA_INT_WPAN0_LED		= BIT(15),
+	OMNIA_INT_WPAN1_LED		= BIT(16),
+	OMNIA_INT_WPAN2_LED		= BIT(17),
+	OMNIA_INT_WAN_LED0		= BIT(18),
+	OMNIA_INT_WAN_LED1		= BIT(19),
+	OMNIA_INT_LAN0_LED0		= BIT(20),
+	OMNIA_INT_LAN0_LED1		= BIT(21),
+	OMNIA_INT_LAN1_LED0		= BIT(22),
+	OMNIA_INT_LAN1_LED1		= BIT(23),
+	OMNIA_INT_LAN2_LED0		= BIT(24),
+	OMNIA_INT_LAN2_LED1		= BIT(25),
+	OMNIA_INT_LAN3_LED0		= BIT(26),
+	OMNIA_INT_LAN3_LED1		= BIT(27),
+	OMNIA_INT_LAN4_LED0		= BIT(28),
+	OMNIA_INT_LAN4_LED1		= BIT(29),
+	OMNIA_INT_LAN5_LED0		= BIT(30),
+	OMNIA_INT_LAN5_LED1		= BIT(31),
+};
+
+enum omnia_cmd_poweroff_e {
+	OMNIA_CMD_POWER_OFF_POWERON_BUTTON	= BIT(0),
+	OMNIA_CMD_POWER_OFF_MAGIC		= 0xdead,
+};
+
+enum omnia_cmd_usb_ovc_prot_e {
+	OMNIA_CMD_xET_USB_OVC_PROT_PORT_MASK	= GENMASK(3, 0),
+	OMNIA_CMD_xET_USB_OVC_PROT_ENABLE	= BIT(4),
+};
+
+#endif /* __TURRIS_OMNIA_MCU_INTERFACE_H */
-- 
2.44.2

[PATCH v11 3/8] platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs

[Marek Behún]

Add support for GPIOs connected to the MCU on the Turris Omnia board.

This includes:
- front button pin
- enable pins for USB regulators
- MiniPCIe / mSATA card presence pins in MiniPCIe port 0
- LED output pins from WAN ethernet PHY, LAN switch and MiniPCIe ports
- on board revisions 32+ also various peripheral resets and another
  voltage regulator enable pin

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 .../sysfs-bus-i2c-devices-turris-omnia-mcu    |   16 +
 drivers/platform/cznic/Kconfig                |   15 +
 drivers/platform/cznic/Makefile               |    1 +
 .../platform/cznic/turris-omnia-mcu-base.c    |    3 +-
 .../platform/cznic/turris-omnia-mcu-gpio.c    | 1071 +++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |   68 ++
 6 files changed, 1173 insertions(+), 1 deletion(-)
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-gpio.c

diff --git a/Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu b/Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu
index 9bc5aad00de0..86360249080c 100644
--- a/Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu
+++ b/Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu
@@ -22,6 +22,22 @@ Description:	(RO) Contains device first MAC address. Each Turris Omnia is
 
 		Format: %pM.
 
+What:		/sys/bus/i2c/devices/<mcu_device>/front_button_mode
+Date:		September 2024
+KernelVersion:	6.11
+Contact:	Marek Behún <kabel@kernel.org>
+Description:	(RW) The front button on the Turris Omnia router can be
+		configured either to change the intensity of all the LEDs on the
+		front panel, or to send the press event to the CPU as an
+		interrupt.
+
+		This file switches between these two modes:
+		- "mcu" makes the button press event be handled by the MCU to
+		  change the LEDs panel intensity.
+		- "cpu" makes the button press event be handled by the CPU.
+
+		Format: %s.
+
 What:		/sys/bus/i2c/devices/<mcu_device>/fw_features
 Date:		September 2024
 KernelVersion:	6.11
diff --git a/drivers/platform/cznic/Kconfig b/drivers/platform/cznic/Kconfig
index db5f4a673d28..d95e7c83c7ae 100644
--- a/drivers/platform/cznic/Kconfig
+++ b/drivers/platform/cznic/Kconfig
@@ -16,9 +16,24 @@ config TURRIS_OMNIA_MCU
 	tristate "Turris Omnia MCU driver"
 	depends on MACH_ARMADA_38X || COMPILE_TEST
 	depends on I2C
+	select GPIOLIB
+	select GPIOLIB_IRQCHIP
 	help
 	  Say Y here to add support for the features implemented by the
 	  microcontroller on the CZ.NIC's Turris Omnia SOHO router.
+	  The features include:
+	  - GPIO pins
+	    - to get front button press events (the front button can be
+	      configured either to generate press events to the CPU or to change
+	      front LEDs panel brightness)
+	    - to enable / disable USB port voltage regulators and to detect
+	      USB overcurrent
+	    - to detect MiniPCIe / mSATA card presence in MiniPCIe port 0
+	    - to configure resets of various peripherals on board revisions 32+
+	    - to enable / disable the VHV voltage regulator to the SOC in order
+	      to be able to program SOC's OTP on board revisions 32+
+	    - to get input from the LED output pins of the WAN ethernet PHY, LAN
+	      switch and MiniPCIe ports
 	  To compile this driver as a module, choose M here; the module will be
 	  called turris-omnia-mcu.
 
diff --git a/drivers/platform/cznic/Makefile b/drivers/platform/cznic/Makefile
index 31adca73bb94..53fd8f1777a3 100644
--- a/drivers/platform/cznic/Makefile
+++ b/drivers/platform/cznic/Makefile
@@ -2,3 +2,4 @@
 
 obj-$(CONFIG_TURRIS_OMNIA_MCU)	+= turris-omnia-mcu.o
 turris-omnia-mcu-y		:= turris-omnia-mcu-base.o
+turris-omnia-mcu-y		+= turris-omnia-mcu-gpio.o
diff --git a/drivers/platform/cznic/turris-omnia-mcu-base.c b/drivers/platform/cznic/turris-omnia-mcu-base.c
index a5ae2ba9abb6..03a5f19c6665 100644
--- a/drivers/platform/cznic/turris-omnia-mcu-base.c
+++ b/drivers/platform/cznic/turris-omnia-mcu-base.c
@@ -196,6 +196,7 @@ static const struct attribute_group omnia_mcu_base_group = {
 
 static const struct attribute_group *omnia_mcu_groups[] = {
 	&omnia_mcu_base_group,
+	&omnia_mcu_gpio_group,
 	NULL
 };
 
@@ -370,7 +371,7 @@ static int omnia_mcu_probe(struct i2c_client *client)
 					     "Cannot read board info\n");
 	}
 
-	return 0;
+	return omnia_mcu_register_gpiochip(mcu);
 }
 
 static const struct of_device_id of_omnia_mcu_match[] = {
diff --git a/drivers/platform/cznic/turris-omnia-mcu-gpio.c b/drivers/platform/cznic/turris-omnia-mcu-gpio.c
new file mode 100644
index 000000000000..bc7965e6c879
--- /dev/null
+++ b/drivers/platform/cznic/turris-omnia-mcu-gpio.c
@@ -0,0 +1,1071 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CZ.NIC's Turris Omnia MCU GPIO and IRQ driver
+ *
+ * 2024 by Marek Behún <kabel@kernel.org>
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/bug.h>
+#include <linux/cleanup.h>
+#include <linux/device.h>
+#include <linux/devm-helpers.h>
+#include <linux/gpio/driver.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/mutex.h>
+#include <linux/sysfs.h>
+#include <linux/turris-omnia-mcu-interface.h>
+#include <linux/types.h>
+#include <linux/workqueue.h>
+#include <asm/unaligned.h>
+
+#include "turris-omnia-mcu.h"
+
+#define OMNIA_CMD_INT_ARG_LEN		8
+#define FRONT_BUTTON_RELEASE_DELAY_MS	50
+
+static const char * const omnia_mcu_gpio_templates[64] = {
+	/* GPIOs with value read from the 16-bit wide status */
+	[4]  = "MiniPCIe0 Card Detect",
+	[5]  = "MiniPCIe0 mSATA Indicator",
+	[6]  = "Front USB3 port over-current",
+	[7]  = "Rear USB3 port over-current",
+	[8]  = "Front USB3 port power",
+	[9]  = "Rear USB3 port power",
+	[12] = "Front Button",
+
+	/* GPIOs with value read from the 32-bit wide extended status */
+	[16] = "SFP nDET",
+	[28] = "MiniPCIe0 LED",
+	[29] = "MiniPCIe1 LED",
+	[30] = "MiniPCIe2 LED",
+	[31] = "MiniPCIe0 PAN LED",
+	[32] = "MiniPCIe1 PAN LED",
+	[33] = "MiniPCIe2 PAN LED",
+	[34] = "WAN PHY LED0",
+	[35] = "WAN PHY LED1",
+	[36] = "LAN switch p0 LED0",
+	[37] = "LAN switch p0 LED1",
+	[38] = "LAN switch p1 LED0",
+	[39] = "LAN switch p1 LED1",
+	[40] = "LAN switch p2 LED0",
+	[41] = "LAN switch p2 LED1",
+	[42] = "LAN switch p3 LED0",
+	[43] = "LAN switch p3 LED1",
+	[44] = "LAN switch p4 LED0",
+	[45] = "LAN switch p4 LED1",
+	[46] = "LAN switch p5 LED0",
+	[47] = "LAN switch p5 LED1",
+
+	/* GPIOs with value read from the 16-bit wide extended control status */
+	[48] = "eMMC nRESET",
+	[49] = "LAN switch nRESET",
+	[50] = "WAN PHY nRESET",
+	[51] = "MiniPCIe0 nPERST",
+	[52] = "MiniPCIe1 nPERST",
+	[53] = "MiniPCIe2 nPERST",
+	[54] = "WAN PHY SFP mux",
+};
+
+struct omnia_gpio {
+	u8 cmd, ctl_cmd, bit, ctl_bit, int_bit;
+	bool has_int;
+	u16 feat, feat_mask;
+};
+
+#define _DEF_GPIO(_cmd, _ctl_cmd, _bit, _ctl_bit, _int_bit, _feat, _feat_mask) \
+	{								\
+		.cmd = _cmd,						\
+		.ctl_cmd = _ctl_cmd,					\
+		.bit = _bit,						\
+		.ctl_bit = _ctl_bit,					\
+		.int_bit = _int_bit,					\
+		.has_int = (_int_bit) != -1,				\
+		.feat = _feat,						\
+		.feat_mask = _feat_mask,				\
+	}
+#define _DEF_GPIO_STS(_name) \
+	_DEF_GPIO(OMNIA_CMD_GET_STATUS_WORD, 0, __bf_shf(OMNIA_STS_ ## _name), \
+		  0, __bf_shf(OMNIA_INT_ ## _name), 0, 0)
+#define _DEF_GPIO_CTL(_name) \
+	_DEF_GPIO(OMNIA_CMD_GET_STATUS_WORD, OMNIA_CMD_GENERAL_CONTROL, \
+		  __bf_shf(OMNIA_STS_ ## _name), __bf_shf(OMNIA_CTL_ ## _name), \
+		  -1, 0, 0)
+#define _DEF_GPIO_EXT_STS(_name, _feat) \
+	_DEF_GPIO(OMNIA_CMD_GET_EXT_STATUS_DWORD, 0, \
+		  __bf_shf(OMNIA_EXT_STS_ ## _name), 0, \
+		  __bf_shf(OMNIA_INT_ ## _name), \
+		  OMNIA_FEAT_ ## _feat | OMNIA_FEAT_EXT_CMDS, \
+		  OMNIA_FEAT_ ## _feat | OMNIA_FEAT_EXT_CMDS)
+#define _DEF_GPIO_EXT_STS_LED(_name, _ledext) \
+	_DEF_GPIO(OMNIA_CMD_GET_EXT_STATUS_DWORD, 0, \
+		  __bf_shf(OMNIA_EXT_STS_ ## _name), 0, \
+		  __bf_shf(OMNIA_INT_ ## _name), \
+		  OMNIA_FEAT_LED_STATE_ ## _ledext, \
+		  OMNIA_FEAT_LED_STATE_EXT_MASK)
+#define _DEF_GPIO_EXT_STS_LEDALL(_name) \
+	_DEF_GPIO(OMNIA_CMD_GET_EXT_STATUS_DWORD, 0, \
+		  __bf_shf(OMNIA_EXT_STS_ ## _name), 0, \
+		  __bf_shf(OMNIA_INT_ ## _name), \
+		  OMNIA_FEAT_LED_STATE_EXT_MASK, 0)
+#define _DEF_GPIO_EXT_CTL(_name, _feat) \
+	_DEF_GPIO(OMNIA_CMD_GET_EXT_CONTROL_STATUS, OMNIA_CMD_EXT_CONTROL, \
+		  __bf_shf(OMNIA_EXT_CTL_ ## _name), \
+		  __bf_shf(OMNIA_EXT_CTL_ ## _name), -1, \
+		  OMNIA_FEAT_ ## _feat | OMNIA_FEAT_EXT_CMDS, \
+		  OMNIA_FEAT_ ## _feat | OMNIA_FEAT_EXT_CMDS)
+#define _DEF_INT(_name) \
+	_DEF_GPIO(0, 0, 0, 0, __bf_shf(OMNIA_INT_ ## _name), 0, 0)
+
+static const struct omnia_gpio omnia_gpios[64] = {
+	/* GPIOs with value read from the 16-bit wide status */
+	[4]  = _DEF_GPIO_STS(CARD_DET),
+	[5]  = _DEF_GPIO_STS(MSATA_IND),
+	[6]  = _DEF_GPIO_STS(USB30_OVC),
+	[7]  = _DEF_GPIO_STS(USB31_OVC),
+	[8]  = _DEF_GPIO_CTL(USB30_PWRON),
+	[9]  = _DEF_GPIO_CTL(USB31_PWRON),
+
+	/* brightness changed interrupt, no GPIO */
+	[11] = _DEF_INT(BRIGHTNESS_CHANGED),
+
+	[12] = _DEF_GPIO_STS(BUTTON_PRESSED),
+
+	/* TRNG interrupt, no GPIO */
+	[13] = _DEF_INT(TRNG),
+
+	/* MESSAGE_SIGNED interrupt, no GPIO */
+	[14] = _DEF_INT(MESSAGE_SIGNED),
+
+	/* GPIOs with value read from the 32-bit wide extended status */
+	[16] = _DEF_GPIO_EXT_STS(SFP_nDET, PERIPH_MCU),
+	[28] = _DEF_GPIO_EXT_STS_LEDALL(WLAN0_MSATA_LED),
+	[29] = _DEF_GPIO_EXT_STS_LEDALL(WLAN1_LED),
+	[30] = _DEF_GPIO_EXT_STS_LEDALL(WLAN2_LED),
+	[31] = _DEF_GPIO_EXT_STS_LED(WPAN0_LED, EXT),
+	[32] = _DEF_GPIO_EXT_STS_LED(WPAN1_LED, EXT),
+	[33] = _DEF_GPIO_EXT_STS_LED(WPAN2_LED, EXT),
+	[34] = _DEF_GPIO_EXT_STS_LEDALL(WAN_LED0),
+	[35] = _DEF_GPIO_EXT_STS_LED(WAN_LED1, EXT_V32),
+	[36] = _DEF_GPIO_EXT_STS_LEDALL(LAN0_LED0),
+	[37] = _DEF_GPIO_EXT_STS_LEDALL(LAN0_LED1),
+	[38] = _DEF_GPIO_EXT_STS_LEDALL(LAN1_LED0),
+	[39] = _DEF_GPIO_EXT_STS_LEDALL(LAN1_LED1),
+	[40] = _DEF_GPIO_EXT_STS_LEDALL(LAN2_LED0),
+	[41] = _DEF_GPIO_EXT_STS_LEDALL(LAN2_LED1),
+	[42] = _DEF_GPIO_EXT_STS_LEDALL(LAN3_LED0),
+	[43] = _DEF_GPIO_EXT_STS_LEDALL(LAN3_LED1),
+	[44] = _DEF_GPIO_EXT_STS_LEDALL(LAN4_LED0),
+	[45] = _DEF_GPIO_EXT_STS_LEDALL(LAN4_LED1),
+	[46] = _DEF_GPIO_EXT_STS_LEDALL(LAN5_LED0),
+	[47] = _DEF_GPIO_EXT_STS_LEDALL(LAN5_LED1),
+
+	/* GPIOs with value read from the 16-bit wide extended control status */
+	[48] = _DEF_GPIO_EXT_CTL(nRES_MMC, PERIPH_MCU),
+	[49] = _DEF_GPIO_EXT_CTL(nRES_LAN, PERIPH_MCU),
+	[50] = _DEF_GPIO_EXT_CTL(nRES_PHY, PERIPH_MCU),
+	[51] = _DEF_GPIO_EXT_CTL(nPERST0, PERIPH_MCU),
+	[52] = _DEF_GPIO_EXT_CTL(nPERST1, PERIPH_MCU),
+	[53] = _DEF_GPIO_EXT_CTL(nPERST2, PERIPH_MCU),
+	[54] = _DEF_GPIO_EXT_CTL(PHY_SFP, PERIPH_MCU),
+};
+
+/* mapping from interrupts to indexes of GPIOs in the omnia_gpios array */
+static const u8 omnia_int_to_gpio_idx[32] = {
+	[__bf_shf(OMNIA_INT_CARD_DET)]			= 4,
+	[__bf_shf(OMNIA_INT_MSATA_IND)]			= 5,
+	[__bf_shf(OMNIA_INT_USB30_OVC)]			= 6,
+	[__bf_shf(OMNIA_INT_USB31_OVC)]			= 7,
+	[__bf_shf(OMNIA_INT_BUTTON_PRESSED)]		= 12,
+	[__bf_shf(OMNIA_INT_TRNG)]			= 13,
+	[__bf_shf(OMNIA_INT_MESSAGE_SIGNED)]		= 14,
+	[__bf_shf(OMNIA_INT_SFP_nDET)]			= 16,
+	[__bf_shf(OMNIA_INT_BRIGHTNESS_CHANGED)]	= 11,
+	[__bf_shf(OMNIA_INT_WLAN0_MSATA_LED)]		= 28,
+	[__bf_shf(OMNIA_INT_WLAN1_LED)]			= 29,
+	[__bf_shf(OMNIA_INT_WLAN2_LED)]			= 30,
+	[__bf_shf(OMNIA_INT_WPAN0_LED)]			= 31,
+	[__bf_shf(OMNIA_INT_WPAN1_LED)]			= 32,
+	[__bf_shf(OMNIA_INT_WPAN2_LED)]			= 33,
+	[__bf_shf(OMNIA_INT_WAN_LED0)]			= 34,
+	[__bf_shf(OMNIA_INT_WAN_LED1)]			= 35,
+	[__bf_shf(OMNIA_INT_LAN0_LED0)]			= 36,
+	[__bf_shf(OMNIA_INT_LAN0_LED1)]			= 37,
+	[__bf_shf(OMNIA_INT_LAN1_LED0)]			= 38,
+	[__bf_shf(OMNIA_INT_LAN1_LED1)]			= 39,
+	[__bf_shf(OMNIA_INT_LAN2_LED0)]			= 40,
+	[__bf_shf(OMNIA_INT_LAN2_LED1)]			= 41,
+	[__bf_shf(OMNIA_INT_LAN3_LED0)]			= 42,
+	[__bf_shf(OMNIA_INT_LAN3_LED1)]			= 43,
+	[__bf_shf(OMNIA_INT_LAN4_LED0)]			= 44,
+	[__bf_shf(OMNIA_INT_LAN4_LED1)]			= 45,
+	[__bf_shf(OMNIA_INT_LAN5_LED0)]			= 46,
+	[__bf_shf(OMNIA_INT_LAN5_LED1)]			= 47,
+};
+
+/* index of PHY_SFP GPIO in the omnia_gpios array */
+#define OMNIA_GPIO_PHY_SFP_OFFSET	54
+
+static int omnia_ctl_cmd_locked(struct omnia_mcu *mcu, u8 cmd, u16 val, u16 mask)
+{
+	unsigned int len;
+	u8 buf[5];
+
+	buf[0] = cmd;
+
+	switch (cmd) {
+	case OMNIA_CMD_GENERAL_CONTROL:
+		buf[1] = val;
+		buf[2] = mask;
+		len = 3;
+		break;
+
+	case OMNIA_CMD_EXT_CONTROL:
+		put_unaligned_le16(val, &buf[1]);
+		put_unaligned_le16(mask, &buf[3]);
+		len = 5;
+		break;
+
+	default:
+		BUG();
+	}
+
+	return omnia_cmd_write(mcu->client, buf, len);
+}
+
+static int omnia_ctl_cmd(struct omnia_mcu *mcu, u8 cmd, u16 val, u16 mask)
+{
+	guard(mutex)(&mcu->lock);
+
+	return omnia_ctl_cmd_locked(mcu, cmd, val, mask);
+}
+
+static int omnia_gpio_request(struct gpio_chip *gc, unsigned int offset)
+{
+	if (!omnia_gpios[offset].cmd)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int omnia_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
+{
+	struct omnia_mcu *mcu = gpiochip_get_data(gc);
+
+	if (offset == OMNIA_GPIO_PHY_SFP_OFFSET) {
+		int val;
+
+		scoped_guard(mutex, &mcu->lock) {
+			val = omnia_cmd_read_bit(mcu->client,
+						 OMNIA_CMD_GET_EXT_CONTROL_STATUS,
+						 OMNIA_EXT_CTL_PHY_SFP_AUTO);
+			if (val < 0)
+				return val;
+		}
+
+		if (val)
+			return GPIO_LINE_DIRECTION_IN;
+
+		return GPIO_LINE_DIRECTION_OUT;
+	}
+
+	if (omnia_gpios[offset].ctl_cmd)
+		return GPIO_LINE_DIRECTION_OUT;
+
+	return GPIO_LINE_DIRECTION_IN;
+}
+
+static int omnia_gpio_direction_input(struct gpio_chip *gc, unsigned int offset)
+{
+	const struct omnia_gpio *gpio = &omnia_gpios[offset];
+	struct omnia_mcu *mcu = gpiochip_get_data(gc);
+
+	if (offset == OMNIA_GPIO_PHY_SFP_OFFSET)
+		return omnia_ctl_cmd(mcu, OMNIA_CMD_EXT_CONTROL,
+				     OMNIA_EXT_CTL_PHY_SFP_AUTO,
+				     OMNIA_EXT_CTL_PHY_SFP_AUTO);
+
+	if (gpio->ctl_cmd)
+		return -ENOTSUPP;
+
+	return 0;
+}
+
+static int omnia_gpio_direction_output(struct gpio_chip *gc,
+				       unsigned int offset, int value)
+{
+	const struct omnia_gpio *gpio = &omnia_gpios[offset];
+	struct omnia_mcu *mcu = gpiochip_get_data(gc);
+	u16 val, mask;
+
+	if (!gpio->ctl_cmd)
+		return -ENOTSUPP;
+
+	mask = BIT(gpio->ctl_bit);
+	val = value ? mask : 0;
+
+	if (offset == OMNIA_GPIO_PHY_SFP_OFFSET)
+		mask |= OMNIA_EXT_CTL_PHY_SFP_AUTO;
+
+	return omnia_ctl_cmd(mcu, gpio->ctl_cmd, val, mask);
+}
+
+static int omnia_gpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+	const struct omnia_gpio *gpio = &omnia_gpios[offset];
+	struct omnia_mcu *mcu = gpiochip_get_data(gc);
+
+	/*
+	 * If firmware does not support the new interrupt API, we are informed
+	 * of every change of the status word by an interrupt from MCU and save
+	 * its value in the interrupt service routine. Simply return the saved
+	 * value.
+	 */
+	if (gpio->cmd == OMNIA_CMD_GET_STATUS_WORD &&
+	    !(mcu->features & OMNIA_FEAT_NEW_INT_API))
+		return test_bit(gpio->bit, &mcu->last_status);
+
+	guard(mutex)(&mcu->lock);
+
+	/*
+	 * If firmware does support the new interrupt API, we may have cached
+	 * the value of a GPIO in the interrupt service routine. If not, read
+	 * the relevant bit now.
+	 */
+	if (gpio->has_int && test_bit(gpio->int_bit, &mcu->is_cached))
+		return test_bit(gpio->int_bit, &mcu->cached);
+
+	return omnia_cmd_read_bit(mcu->client, gpio->cmd, BIT(gpio->bit));
+}
+
+static unsigned long *
+_relevant_field_for_sts_cmd(u8 cmd, unsigned long *sts, unsigned long *ext_sts,
+			    unsigned long *ext_ctl)
+{
+	switch (cmd) {
+	case OMNIA_CMD_GET_STATUS_WORD:
+		return sts;
+	case OMNIA_CMD_GET_EXT_STATUS_DWORD:
+		return ext_sts;
+	case OMNIA_CMD_GET_EXT_CONTROL_STATUS:
+		return ext_ctl;
+	default:
+		return NULL;
+	}
+}
+
+static int omnia_gpio_get_multiple(struct gpio_chip *gc, unsigned long *mask,
+				   unsigned long *bits)
+{
+	unsigned long sts = 0, ext_sts = 0, ext_ctl = 0, *field;
+	struct omnia_mcu *mcu = gpiochip_get_data(gc);
+	struct i2c_client *client = mcu->client;
+	int err, i;
+
+	/* determine which bits to read from the 3 possible commands */
+	for_each_set_bit(i, mask, ARRAY_SIZE(omnia_gpios)) {
+		field = _relevant_field_for_sts_cmd(omnia_gpios[i].cmd,
+						    &sts, &ext_sts, &ext_ctl);
+
+		if (field)
+			__set_bit(omnia_gpios[i].bit, field);
+	}
+
+	guard(mutex)(&mcu->lock);
+
+	if (mcu->features & OMNIA_FEAT_NEW_INT_API) {
+		/* read relevant bits from status */
+		err = omnia_cmd_read_bits(client, OMNIA_CMD_GET_STATUS_WORD,
+					  sts, &sts);
+		if (err)
+			return err;
+	} else {
+		/*
+		 * Use status word value cached in the interrupt service routine
+		 * if firmware does not support the new interrupt API.
+		 */
+		sts = mcu->last_status;
+	}
+
+	/* read relevant bits from extended status */
+	err = omnia_cmd_read_bits(client, OMNIA_CMD_GET_EXT_STATUS_DWORD,
+				  ext_sts, &ext_sts);
+	if (err)
+		return err;
+
+	/* read relevant bits from extended control */
+	err = omnia_cmd_read_bits(client, OMNIA_CMD_GET_EXT_CONTROL_STATUS,
+				  ext_ctl, &ext_ctl);
+	if (err)
+		return err;
+
+	/* assign relevant bits in result */
+	for_each_set_bit(i, mask, ARRAY_SIZE(omnia_gpios)) {
+		field = _relevant_field_for_sts_cmd(omnia_gpios[i].cmd,
+						    &sts, &ext_sts, &ext_ctl);
+
+		if (field)
+			__assign_bit(i, bits,
+				     test_bit(omnia_gpios[i].bit, field));
+	}
+
+	return 0;
+}
+
+static void omnia_gpio_set(struct gpio_chip *gc, unsigned int offset, int value)
+{
+	const struct omnia_gpio *gpio = &omnia_gpios[offset];
+	struct omnia_mcu *mcu = gpiochip_get_data(gc);
+	u16 val, mask;
+
+	if (!gpio->ctl_cmd)
+		return;
+
+	mask = BIT(gpio->ctl_bit);
+	val = value ? mask : 0;
+
+	omnia_ctl_cmd(mcu, gpio->ctl_cmd, val, mask);
+}
+
+static void omnia_gpio_set_multiple(struct gpio_chip *gc, unsigned long *mask,
+				    unsigned long *bits)
+{
+	unsigned long ctl = 0, ctl_mask = 0, ext_ctl = 0, ext_ctl_mask = 0;
+	struct omnia_mcu *mcu = gpiochip_get_data(gc);
+	int i;
+
+	for_each_set_bit(i, mask, ARRAY_SIZE(omnia_gpios)) {
+		unsigned long *field, *field_mask;
+		u8 bit = omnia_gpios[i].ctl_bit;
+
+		switch (omnia_gpios[i].ctl_cmd) {
+		case OMNIA_CMD_GENERAL_CONTROL:
+			field = &ctl;
+			field_mask = &ctl_mask;
+			break;
+		case OMNIA_CMD_EXT_CONTROL:
+			field = &ext_ctl;
+			field_mask = &ext_ctl_mask;
+			break;
+		default:
+			field = field_mask = NULL;
+			break;
+		}
+
+		if (field) {
+			__set_bit(bit, field_mask);
+			__assign_bit(bit, field, test_bit(i, bits));
+		}
+	}
+
+	guard(mutex)(&mcu->lock);
+
+	if (ctl_mask)
+		omnia_ctl_cmd_locked(mcu, OMNIA_CMD_GENERAL_CONTROL,
+				     ctl, ctl_mask);
+
+	if (ext_ctl_mask)
+		omnia_ctl_cmd_locked(mcu, OMNIA_CMD_EXT_CONTROL,
+				     ext_ctl, ext_ctl_mask);
+}
+
+static bool omnia_gpio_available(struct omnia_mcu *mcu,
+				 const struct omnia_gpio *gpio)
+{
+	if (gpio->feat_mask)
+		return (mcu->features & gpio->feat_mask) == gpio->feat;
+
+	if (gpio->feat)
+		return mcu->features & gpio->feat;
+
+	return true;
+}
+
+static int omnia_gpio_init_valid_mask(struct gpio_chip *gc,
+				      unsigned long *valid_mask,
+				      unsigned int ngpios)
+{
+	struct omnia_mcu *mcu = gpiochip_get_data(gc);
+
+	for (unsigned int i = 0; i < ngpios; i++) {
+		const struct omnia_gpio *gpio = &omnia_gpios[i];
+
+		if (gpio->cmd || gpio->has_int)
+			__assign_bit(i, valid_mask,
+				     omnia_gpio_available(mcu, gpio));
+		else
+			__clear_bit(i, valid_mask);
+	}
+
+	return 0;
+}
+
+static int omnia_gpio_of_xlate(struct gpio_chip *gc,
+			       const struct of_phandle_args *gpiospec,
+			       u32 *flags)
+{
+	u32 bank, gpio;
+
+	if (WARN_ON(gpiospec->args_count != 3))
+		return -EINVAL;
+
+	if (flags)
+		*flags = gpiospec->args[2];
+
+	bank = gpiospec->args[0];
+	gpio = gpiospec->args[1];
+
+	switch (bank) {
+	case 0:
+		return gpio < 16 ? gpio : -EINVAL;
+	case 1:
+		return gpio < 32 ? 16 + gpio : -EINVAL;
+	case 2:
+		return gpio < 16 ? 48 + gpio : -EINVAL;
+	default:
+		return -EINVAL;
+	}
+}
+
+static void omnia_irq_shutdown(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct omnia_mcu *mcu = gpiochip_get_data(gc);
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+	u8 bit = omnia_gpios[hwirq].int_bit;
+
+	__clear_bit(bit, &mcu->rising);
+	__clear_bit(bit, &mcu->falling);
+}
+
+static void omnia_irq_mask(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct omnia_mcu *mcu = gpiochip_get_data(gc);
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+	u8 bit = omnia_gpios[hwirq].int_bit;
+
+	if (!omnia_gpios[hwirq].cmd)
+		__clear_bit(bit, &mcu->rising);
+	__clear_bit(bit, &mcu->mask);
+	gpiochip_disable_irq(gc, hwirq);
+}
+
+static void omnia_irq_unmask(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct omnia_mcu *mcu = gpiochip_get_data(gc);
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+	u8 bit = omnia_gpios[hwirq].int_bit;
+
+	gpiochip_enable_irq(gc, hwirq);
+	__set_bit(bit, &mcu->mask);
+	if (!omnia_gpios[hwirq].cmd)
+		__set_bit(bit, &mcu->rising);
+}
+
+static int omnia_irq_set_type(struct irq_data *d, unsigned int type)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct omnia_mcu *mcu = gpiochip_get_data(gc);
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+	struct device *dev = &mcu->client->dev;
+	u8 bit = omnia_gpios[hwirq].int_bit;
+
+	if (!(type & IRQ_TYPE_EDGE_BOTH)) {
+		dev_err(dev, "irq %u: unsupported type %u\n", d->irq, type);
+		return -EINVAL;
+	}
+
+	__assign_bit(bit, &mcu->rising, type & IRQ_TYPE_EDGE_RISING);
+	__assign_bit(bit, &mcu->falling, type & IRQ_TYPE_EDGE_FALLING);
+
+	return 0;
+}
+
+static void omnia_irq_bus_lock(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct omnia_mcu *mcu = gpiochip_get_data(gc);
+
+	/* nothing to do if MCU firmware does not support new interrupt API */
+	if (!(mcu->features & OMNIA_FEAT_NEW_INT_API))
+		return;
+
+	mutex_lock(&mcu->lock);
+}
+
+/**
+ * omnia_mask_interleave - Interleaves the bytes from @rising and @falling
+ *	@dst: the destination u8 array of interleaved bytes
+ *	@rising: rising mask
+ *	@falling: falling mask
+ *
+ * Interleaves the little-endian bytes from @rising and @falling words.
+ *
+ * If @rising = (r0, r1, r2, r3) and @falling = (f0, f1, f2, f3), the result is
+ * @dst = (r0, f0, r1, f1, r2, f2, r3, f3).
+ *
+ * The MCU receives an interrupt mask and reports a pending interrupt bitmap in
+ * this interleaved format. The rationale behind this is that the low-indexed
+ * bits are more important - in many cases, the user will be interested only in
+ * interrupts with indexes 0 to 7, and so the system can stop reading after
+ * first 2 bytes (r0, f0), to save time on the slow I2C bus.
+ *
+ * Feel free to remove this function and its inverse, omnia_mask_deinterleave,
+ * and use an appropriate bitmap_*() function once such a function exists.
+ */
+static void
+omnia_mask_interleave(u8 *dst, unsigned long rising, unsigned long falling)
+{
+	for (int i = 0; i < sizeof(u32); ++i) {
+		dst[2 * i] = rising >> (8 * i);
+		dst[2 * i + 1] = falling >> (8 * i);
+	}
+}
+
+/**
+ * omnia_mask_deinterleave - Deinterleaves the bytes into @rising and @falling
+ *	@src: the source u8 array containing the interleaved bytes
+ *	@rising: pointer where to store the rising mask gathered from @src
+ *	@falling: pointer where to store the falling mask gathered from @src
+ *
+ * This is the inverse function to omnia_mask_interleave.
+ */
+static void omnia_mask_deinterleave(const u8 *src, unsigned long *rising,
+				    unsigned long *falling)
+{
+	*rising = *falling = 0;
+
+	for (int i = 0; i < sizeof(u32); ++i) {
+		*rising |= src[2 * i] << (8 * i);
+		*falling |= src[2 * i + 1] << (8 * i);
+	}
+}
+
+static void omnia_irq_bus_sync_unlock(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct omnia_mcu *mcu = gpiochip_get_data(gc);
+	struct device *dev = &mcu->client->dev;
+	u8 cmd[1 + OMNIA_CMD_INT_ARG_LEN];
+	unsigned long rising, falling;
+	int err;
+
+	/* nothing to do if MCU firmware does not support new interrupt API */
+	if (!(mcu->features & OMNIA_FEAT_NEW_INT_API))
+		return;
+
+	cmd[0] = OMNIA_CMD_SET_INT_MASK;
+
+	rising = mcu->rising & mcu->mask;
+	falling = mcu->falling & mcu->mask;
+
+	/* interleave the rising and falling bytes into the command arguments */
+	omnia_mask_interleave(&cmd[1], rising, falling);
+
+	dev_dbg(dev, "set int mask %8ph\n", &cmd[1]);
+
+	err = omnia_cmd_write(mcu->client, cmd, sizeof(cmd));
+	if (err) {
+		dev_err(dev, "Cannot set mask: %d\n", err);
+		goto unlock;
+	}
+
+	/*
+	 * Remember which GPIOs have both rising and falling interrupts enabled.
+	 * For those we will cache their value so that .get() method is faster.
+	 * We also need to forget cached values of GPIOs that aren't cached
+	 * anymore.
+	 */
+	mcu->both = rising & falling;
+	mcu->is_cached &= mcu->both;
+
+unlock:
+	mutex_unlock(&mcu->lock);
+}
+
+static const struct irq_chip omnia_mcu_irq_chip = {
+	.name			= "Turris Omnia MCU interrupts",
+	.irq_shutdown		= omnia_irq_shutdown,
+	.irq_mask		= omnia_irq_mask,
+	.irq_unmask		= omnia_irq_unmask,
+	.irq_set_type		= omnia_irq_set_type,
+	.irq_bus_lock		= omnia_irq_bus_lock,
+	.irq_bus_sync_unlock	= omnia_irq_bus_sync_unlock,
+	.flags			= IRQCHIP_IMMUTABLE,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
+static void omnia_irq_init_valid_mask(struct gpio_chip *gc,
+				      unsigned long *valid_mask,
+				      unsigned int ngpios)
+{
+	struct omnia_mcu *mcu = gpiochip_get_data(gc);
+
+	for (unsigned int i = 0; i < ngpios; i++) {
+		const struct omnia_gpio *gpio = &omnia_gpios[i];
+
+		if (gpio->has_int)
+			__assign_bit(i, valid_mask,
+				     omnia_gpio_available(mcu, gpio));
+		else
+			__clear_bit(i, valid_mask);
+	}
+}
+
+static int omnia_irq_init_hw(struct gpio_chip *gc)
+{
+	struct omnia_mcu *mcu = gpiochip_get_data(gc);
+	u8 cmd[1 + OMNIA_CMD_INT_ARG_LEN] = {};
+
+	cmd[0] = OMNIA_CMD_SET_INT_MASK;
+
+	return omnia_cmd_write(mcu->client, cmd, sizeof(cmd));
+}
+
+/*
+ * Determine how many bytes we need to read from the reply to the
+ * OMNIA_CMD_GET_INT_AND_CLEAR command in order to retrieve all unmasked
+ * interrupts.
+ */
+static unsigned int
+omnia_irq_compute_pending_length(unsigned long rising, unsigned long falling)
+{
+	return max(omnia_compute_reply_length(rising, true, 0),
+		   omnia_compute_reply_length(falling, true, 1));
+}
+
+static bool omnia_irq_read_pending_new(struct omnia_mcu *mcu,
+				       unsigned long *pending)
+{
+	struct device *dev = &mcu->client->dev;
+	u8 reply[OMNIA_CMD_INT_ARG_LEN] = {};
+	unsigned long rising, falling;
+	unsigned int len;
+	int err;
+
+	len = omnia_irq_compute_pending_length(mcu->rising & mcu->mask,
+					       mcu->falling & mcu->mask);
+	if (!len)
+		return false;
+
+	guard(mutex)(&mcu->lock);
+
+	err = omnia_cmd_read(mcu->client, OMNIA_CMD_GET_INT_AND_CLEAR, reply,
+			     len);
+	if (err) {
+		dev_err(dev, "Cannot read pending IRQs: %d\n", err);
+		return false;
+	}
+
+	/* deinterleave the reply bytes into rising and falling */
+	omnia_mask_deinterleave(reply, &rising, &falling);
+
+	rising &= mcu->mask;
+	falling &= mcu->mask;
+	*pending = rising | falling;
+
+	/* cache values for GPIOs that have both edges enabled */
+	mcu->is_cached &= ~(rising & falling);
+	mcu->is_cached |= mcu->both & (rising ^ falling);
+	mcu->cached = (mcu->cached | rising) & ~falling;
+
+	return true;
+}
+
+static int omnia_read_status_word_old_fw(struct omnia_mcu *mcu,
+					 unsigned long *status)
+{
+	u16 raw_status;
+	int err;
+
+	err = omnia_cmd_read_u16(mcu->client, OMNIA_CMD_GET_STATUS_WORD,
+				 &raw_status);
+	if (err)
+		return err;
+
+	/*
+	 * Old firmware has a bug wherein it never resets the USB port
+	 * overcurrent bits back to zero. Ignore them.
+	 */
+	*status = raw_status & ~(OMNIA_STS_USB30_OVC | OMNIA_STS_USB31_OVC);
+
+	return 0;
+}
+
+static void button_release_emul_fn(struct work_struct *work)
+{
+	struct omnia_mcu *mcu = container_of(to_delayed_work(work),
+					     struct omnia_mcu,
+					     button_release_emul_work);
+
+	mcu->button_pressed_emul = false;
+	generic_handle_irq_safe(mcu->client->irq);
+}
+
+static void
+fill_int_from_sts(unsigned long *rising, unsigned long *falling,
+		  unsigned long rising_sts, unsigned long falling_sts,
+		  unsigned long sts_bit, unsigned long int_bit)
+{
+	if (rising_sts & sts_bit)
+		*rising |= int_bit;
+	if (falling_sts & sts_bit)
+		*falling |= int_bit;
+}
+
+static bool omnia_irq_read_pending_old(struct omnia_mcu *mcu,
+				       unsigned long *pending)
+{
+	unsigned long status, rising_sts, falling_sts, rising, falling;
+	struct device *dev = &mcu->client->dev;
+	int err;
+
+	guard(mutex)(&mcu->lock);
+
+	err = omnia_read_status_word_old_fw(mcu, &status);
+	if (err) {
+		dev_err(dev, "Cannot read pending IRQs: %d\n", err);
+		return false;
+	}
+
+	/*
+	 * The old firmware triggers an interrupt whenever status word changes,
+	 * but does not inform about which bits rose or fell. We need to compute
+	 * this here by comparing with the last status word value.
+	 *
+	 * The OMNIA_STS_BUTTON_PRESSED bit needs special handling, because the
+	 * old firmware clears the OMNIA_STS_BUTTON_PRESSED bit on successful
+	 * completion of the OMNIA_CMD_GET_STATUS_WORD command, resulting in
+	 * another interrupt:
+	 * - first we get an interrupt, we read the status word where
+	 *   OMNIA_STS_BUTTON_PRESSED is present,
+	 * - MCU clears the OMNIA_STS_BUTTON_PRESSED bit because we read the
+	 *   status word,
+	 * - we get another interrupt because the status word changed again
+	 *   (the OMNIA_STS_BUTTON_PRESSED bit was cleared).
+	 *
+	 * The gpiolib-cdev, gpiolib-sysfs and gpio-keys input driver all call
+	 * the gpiochip's .get() method after an edge event on a requested GPIO
+	 * occurs.
+	 *
+	 * We ensure that the .get() method reads 1 for the button GPIO for some
+	 * time.
+	 */
+
+	if (status & OMNIA_STS_BUTTON_PRESSED) {
+		mcu->button_pressed_emul = true;
+		mod_delayed_work(system_wq, &mcu->button_release_emul_work,
+				 msecs_to_jiffies(FRONT_BUTTON_RELEASE_DELAY_MS));
+	} else if (mcu->button_pressed_emul) {
+		status |= OMNIA_STS_BUTTON_PRESSED;
+	}
+
+	rising_sts = ~mcu->last_status & status;
+	falling_sts = mcu->last_status & ~status;
+
+	mcu->last_status = status;
+
+	/*
+	 * Fill in the relevant interrupt bits from status bits for CARD_DET,
+	 * MSATA_IND and BUTTON_PRESSED.
+	 */
+	rising = 0;
+	falling = 0;
+	fill_int_from_sts(&rising, &falling, rising_sts, falling_sts,
+			  OMNIA_STS_CARD_DET, OMNIA_INT_CARD_DET);
+	fill_int_from_sts(&rising, &falling, rising_sts, falling_sts,
+			  OMNIA_STS_MSATA_IND, OMNIA_INT_MSATA_IND);
+	fill_int_from_sts(&rising, &falling, rising_sts, falling_sts,
+			  OMNIA_STS_BUTTON_PRESSED, OMNIA_INT_BUTTON_PRESSED);
+
+	/* Use only bits that are enabled */
+	rising &= mcu->rising & mcu->mask;
+	falling &= mcu->falling & mcu->mask;
+	*pending = rising | falling;
+
+	return true;
+}
+
+static bool omnia_irq_read_pending(struct omnia_mcu *mcu,
+				   unsigned long *pending)
+{
+	if (mcu->features & OMNIA_FEAT_NEW_INT_API)
+		return omnia_irq_read_pending_new(mcu, pending);
+	else
+		return omnia_irq_read_pending_old(mcu, pending);
+}
+
+static irqreturn_t omnia_irq_thread_handler(int irq, void *dev_id)
+{
+	struct omnia_mcu *mcu = dev_id;
+	struct irq_domain *domain;
+	unsigned long pending;
+	int i;
+
+	if (!omnia_irq_read_pending(mcu, &pending))
+		return IRQ_NONE;
+
+	domain = mcu->gc.irq.domain;
+
+	for_each_set_bit(i, &pending, 32) {
+		unsigned int nested_irq;
+
+		nested_irq = irq_find_mapping(domain, omnia_int_to_gpio_idx[i]);
+
+		handle_nested_irq(nested_irq);
+	}
+
+	return IRQ_RETVAL(pending);
+}
+
+static const char * const front_button_modes[] = { "mcu", "cpu" };
+
+static ssize_t front_button_mode_show(struct device *dev,
+				      struct device_attribute *a, char *buf)
+{
+	struct omnia_mcu *mcu = dev_get_drvdata(dev);
+	int val;
+
+	if (mcu->features & OMNIA_FEAT_NEW_INT_API) {
+		val = omnia_cmd_read_bit(mcu->client, OMNIA_CMD_GET_STATUS_WORD,
+					 OMNIA_STS_BUTTON_MODE);
+		if (val < 0)
+			return val;
+	} else {
+		val = !!(mcu->last_status & OMNIA_STS_BUTTON_MODE);
+	}
+
+	return sysfs_emit(buf, "%s\n", front_button_modes[val]);
+}
+
+static ssize_t front_button_mode_store(struct device *dev,
+				       struct device_attribute *a,
+				       const char *buf, size_t count)
+{
+	struct omnia_mcu *mcu = dev_get_drvdata(dev);
+	int err, i;
+
+	i = sysfs_match_string(front_button_modes, buf);
+	if (i < 0)
+		return i;
+
+	err = omnia_ctl_cmd_locked(mcu, OMNIA_CMD_GENERAL_CONTROL,
+				   i ? OMNIA_CTL_BUTTON_MODE : 0,
+				   OMNIA_CTL_BUTTON_MODE);
+	if (err)
+		return err;
+
+	return count;
+}
+static DEVICE_ATTR_RW(front_button_mode);
+
+static struct attribute *omnia_mcu_gpio_attrs[] = {
+	&dev_attr_front_button_mode.attr,
+	NULL
+};
+
+const struct attribute_group omnia_mcu_gpio_group = {
+	.attrs = omnia_mcu_gpio_attrs,
+};
+
+int omnia_mcu_register_gpiochip(struct omnia_mcu *mcu)
+{
+	bool new_api = mcu->features & OMNIA_FEAT_NEW_INT_API;
+	struct device *dev = &mcu->client->dev;
+	unsigned long irqflags;
+	int err;
+
+	err = devm_mutex_init(dev, &mcu->lock);
+	if (err)
+		return err;
+
+	mcu->gc.request = omnia_gpio_request;
+	mcu->gc.get_direction = omnia_gpio_get_direction;
+	mcu->gc.direction_input = omnia_gpio_direction_input;
+	mcu->gc.direction_output = omnia_gpio_direction_output;
+	mcu->gc.get = omnia_gpio_get;
+	mcu->gc.get_multiple = omnia_gpio_get_multiple;
+	mcu->gc.set = omnia_gpio_set;
+	mcu->gc.set_multiple = omnia_gpio_set_multiple;
+	mcu->gc.init_valid_mask = omnia_gpio_init_valid_mask;
+	mcu->gc.can_sleep = true;
+	mcu->gc.names = omnia_mcu_gpio_templates;
+	mcu->gc.base = -1;
+	mcu->gc.ngpio = ARRAY_SIZE(omnia_gpios);
+	mcu->gc.label = "Turris Omnia MCU GPIOs";
+	mcu->gc.parent = dev;
+	mcu->gc.owner = THIS_MODULE;
+	mcu->gc.of_gpio_n_cells = 3;
+	mcu->gc.of_xlate = omnia_gpio_of_xlate;
+
+	gpio_irq_chip_set_chip(&mcu->gc.irq, &omnia_mcu_irq_chip);
+	/* This will let us handle the parent IRQ in the driver */
+	mcu->gc.irq.parent_handler = NULL;
+	mcu->gc.irq.num_parents = 0;
+	mcu->gc.irq.parents = NULL;
+	mcu->gc.irq.default_type = IRQ_TYPE_NONE;
+	mcu->gc.irq.handler = handle_bad_irq;
+	mcu->gc.irq.threaded = true;
+	if (new_api)
+		mcu->gc.irq.init_hw = omnia_irq_init_hw;
+	mcu->gc.irq.init_valid_mask = omnia_irq_init_valid_mask;
+
+	err = devm_gpiochip_add_data(dev, &mcu->gc, mcu);
+	if (err)
+		return dev_err_probe(dev, err, "Cannot add GPIO chip\n");
+
+	/*
+	 * Before requesting the interrupt, if firmware does not support the new
+	 * interrupt API, we need to cache the value of the status word, so that
+	 * when it changes, we may compare the new value with the cached one in
+	 * the interrupt handler.
+	 */
+	if (!new_api) {
+		err = omnia_read_status_word_old_fw(mcu, &mcu->last_status);
+		if (err)
+			return dev_err_probe(dev, err,
+					     "Cannot read status word\n");
+
+		INIT_DELAYED_WORK(&mcu->button_release_emul_work,
+				  button_release_emul_fn);
+	}
+
+	irqflags = IRQF_ONESHOT;
+	if (new_api)
+		irqflags |= IRQF_TRIGGER_LOW;
+	else
+		irqflags |= IRQF_TRIGGER_FALLING;
+
+	err = devm_request_threaded_irq(dev, mcu->client->irq, NULL,
+					omnia_irq_thread_handler, irqflags,
+					"turris-omnia-mcu", mcu);
+	if (err)
+		return dev_err_probe(dev, err, "Cannot request IRQ\n");
+
+	if (!new_api) {
+		/*
+		 * The button_release_emul_work has to be initialized before the
+		 * thread is requested, and on driver remove it needs to be
+		 * canceled before the thread is freed. Therefore we can't use
+		 * devm_delayed_work_autocancel() directly, because the order
+		 *   devm_delayed_work_autocancel();
+		 *   devm_request_threaded_irq();
+		 * would cause improper release order:
+		 *   free_irq();
+		 *   cancel_delayed_work_sync();
+		 * Instead we first initialize the work above, and only now
+		 * after IRQ is requested we add the work devm action.
+		 */
+		err = devm_add_action(dev, devm_delayed_work_drop,
+				      &mcu->button_release_emul_work);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
diff --git a/drivers/platform/cznic/turris-omnia-mcu.h b/drivers/platform/cznic/turris-omnia-mcu.h
index 3d0daa6f13ef..3d2dd0054499 100644
--- a/drivers/platform/cznic/turris-omnia-mcu.h
+++ b/drivers/platform/cznic/turris-omnia-mcu.h
@@ -8,8 +8,12 @@
 #ifndef __TURRIS_OMNIA_MCU_H
 #define __TURRIS_OMNIA_MCU_H
 
+#include <linux/bitops.h>
+#include <linux/gpio/driver.h>
 #include <linux/if_ether.h>
+#include <linux/mutex.h>
 #include <linux/types.h>
+#include <linux/workqueue.h>
 #include <asm/byteorder.h>
 
 struct i2c_client;
@@ -23,18 +27,78 @@ struct omnia_mcu {
 	u64 board_serial_number;
 	u8 board_first_mac[ETH_ALEN];
 	u8 board_revision;
+
+	/* GPIO chip */
+	struct gpio_chip gc;
+	struct mutex lock;
+	unsigned long mask, rising, falling, both, cached, is_cached;
+	/* Old MCU firmware handling needs the following */
+	struct delayed_work button_release_emul_work;
+	unsigned long last_status;
+	bool button_pressed_emul;
 };
 
 int omnia_cmd_write_read(const struct i2c_client *client,
 			 void *cmd, unsigned int cmd_len,
 			 void *reply, unsigned int reply_len);
 
+static inline int omnia_cmd_write(const struct i2c_client *client, void *cmd,
+				  unsigned int len)
+{
+	return omnia_cmd_write_read(client, cmd, len, NULL, 0);
+}
+
 static inline int omnia_cmd_read(const struct i2c_client *client, u8 cmd,
 				 void *reply, unsigned int len)
 {
 	return omnia_cmd_write_read(client, &cmd, 1, reply, len);
 }
 
+static inline unsigned int
+omnia_compute_reply_length(unsigned long mask, bool interleaved,
+			   unsigned int offset)
+{
+	if (!mask)
+		return 0;
+
+	return ((__fls(mask) >> 3) << interleaved) + 1 + offset;
+}
+
+/* Returns 0 on success */
+static inline int omnia_cmd_read_bits(const struct i2c_client *client, u8 cmd,
+				      unsigned long bits, unsigned long *dst)
+{
+	__le32 reply;
+	int err;
+
+	if (!bits) {
+		*dst = 0;
+		return 0;
+	}
+
+	err = omnia_cmd_read(client, cmd, &reply,
+			     omnia_compute_reply_length(bits, false, 0));
+	if (err)
+		return err;
+
+	*dst = le32_to_cpu(reply) & bits;
+
+	return 0;
+}
+
+static inline int omnia_cmd_read_bit(const struct i2c_client *client, u8 cmd,
+				     unsigned long bit)
+{
+	unsigned long reply;
+	int err;
+
+	err = omnia_cmd_read_bits(client, cmd, bit, &reply);
+	if (err)
+		return err;
+
+	return !!reply;
+}
+
 static inline int omnia_cmd_read_u32(const struct i2c_client *client, u8 cmd,
 				     u32 *dst)
 {
@@ -71,4 +135,8 @@ static inline int omnia_cmd_read_u8(const struct i2c_client *client, u8 cmd,
 	return omnia_cmd_read(client, cmd, reply, sizeof(*reply));
 }
 
+extern const struct attribute_group omnia_mcu_gpio_group;
+
+int omnia_mcu_register_gpiochip(struct omnia_mcu *mcu);
+
 #endif /* __TURRIS_OMNIA_MCU_H */
-- 
2.44.2

[PATCH v11 4/8] platform: cznic: turris-omnia-mcu: Add support for poweroff and wakeup

[Marek Behún]

Add support for true board poweroff (MCU can disable all unnecessary
voltage regulators) and wakeup at a specified time, implemented via a
RTC driver so that the rtcwake utility can be used to configure it.

Signed-off-by: Marek Behún <kabel@kernel.org>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 .../sysfs-bus-i2c-devices-turris-omnia-mcu    |  16 ++
 drivers/platform/cznic/Kconfig                |   4 +
 drivers/platform/cznic/Makefile               |   1 +
 .../platform/cznic/turris-omnia-mcu-base.c    |   5 +
 .../cznic/turris-omnia-mcu-sys-off-wakeup.c   | 257 ++++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |  20 ++
 6 files changed, 303 insertions(+)
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c

diff --git a/Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu b/Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu
index 86360249080c..307a55f599cb 100644
--- a/Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu
+++ b/Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu
@@ -38,6 +38,22 @@ Description:	(RW) The front button on the Turris Omnia router can be
 
 		Format: %s.
 
+What:		/sys/bus/i2c/devices/<mcu_device>/front_button_poweron
+Date:		September 2024
+KernelVersion:	6.11
+Contact:	Marek Behún <kabel@kernel.org>
+Description:	(RW) Newer versions of the microcontroller firmware of the
+		Turris Omnia router support powering off the router into true
+		low power mode. The router can be powered on by pressing the
+		front button.
+
+		This file configures whether front button power on is enabled.
+
+		This file is present only if the power off feature is supported
+		by the firmware.
+
+		Format: %i.
+
 What:		/sys/bus/i2c/devices/<mcu_device>/fw_features
 Date:		September 2024
 KernelVersion:	6.11
diff --git a/drivers/platform/cznic/Kconfig b/drivers/platform/cznic/Kconfig
index d95e7c83c7ae..c1e719235517 100644
--- a/drivers/platform/cznic/Kconfig
+++ b/drivers/platform/cznic/Kconfig
@@ -18,10 +18,14 @@ config TURRIS_OMNIA_MCU
 	depends on I2C
 	select GPIOLIB
 	select GPIOLIB_IRQCHIP
+	select RTC_CLASS
 	help
 	  Say Y here to add support for the features implemented by the
 	  microcontroller on the CZ.NIC's Turris Omnia SOHO router.
 	  The features include:
+	  - board poweroff into true low power mode (with voltage regulators
+	    disabled) and the ability to configure wake up from this mode (via
+	    rtcwake)
 	  - GPIO pins
 	    - to get front button press events (the front button can be
 	      configured either to generate press events to the CPU or to change
diff --git a/drivers/platform/cznic/Makefile b/drivers/platform/cznic/Makefile
index 53fd8f1777a3..a185ef882e44 100644
--- a/drivers/platform/cznic/Makefile
+++ b/drivers/platform/cznic/Makefile
@@ -3,3 +3,4 @@
 obj-$(CONFIG_TURRIS_OMNIA_MCU)	+= turris-omnia-mcu.o
 turris-omnia-mcu-y		:= turris-omnia-mcu-base.o
 turris-omnia-mcu-y		+= turris-omnia-mcu-gpio.o
+turris-omnia-mcu-y		+= turris-omnia-mcu-sys-off-wakeup.o
diff --git a/drivers/platform/cznic/turris-omnia-mcu-base.c b/drivers/platform/cznic/turris-omnia-mcu-base.c
index 03a5f19c6665..479f355f730a 100644
--- a/drivers/platform/cznic/turris-omnia-mcu-base.c
+++ b/drivers/platform/cznic/turris-omnia-mcu-base.c
@@ -197,6 +197,7 @@ static const struct attribute_group omnia_mcu_base_group = {
 static const struct attribute_group *omnia_mcu_groups[] = {
 	&omnia_mcu_base_group,
 	&omnia_mcu_gpio_group,
+	&omnia_mcu_poweroff_group,
 	NULL
 };
 
@@ -371,6 +372,10 @@ static int omnia_mcu_probe(struct i2c_client *client)
 					     "Cannot read board info\n");
 	}
 
+	err = omnia_mcu_register_sys_off_and_wakeup(mcu);
+	if (err)
+		return err;
+
 	return omnia_mcu_register_gpiochip(mcu);
 }
 
diff --git a/drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c b/drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c
new file mode 100644
index 000000000000..272664b1b77d
--- /dev/null
+++ b/drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c
@@ -0,0 +1,257 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CZ.NIC's Turris Omnia MCU system off and RTC wakeup driver
+ *
+ * This is not a true RTC driver (in the sense that it does not provide a
+ * real-time clock), rather the MCU implements a wakeup from powered off state
+ * at a specified time relative to MCU boot, and we expose this feature via RTC
+ * alarm, so that it can be used via the rtcwake command, which is the standard
+ * Linux command for this.
+ *
+ * 2024 by Marek Behún <kabel@kernel.org>
+ */
+
+#include <linux/crc32.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/reboot.h>
+#include <linux/rtc.h>
+#include <linux/sysfs.h>
+#include <linux/turris-omnia-mcu-interface.h>
+#include <linux/types.h>
+
+#include "turris-omnia-mcu.h"
+
+static int omnia_get_uptime_wakeup(const struct i2c_client *client, u32 *uptime,
+				   u32 *wakeup)
+{
+	__le32 reply[2];
+	int err;
+
+	err = omnia_cmd_read(client, OMNIA_CMD_GET_UPTIME_AND_WAKEUP, reply,
+			     sizeof(reply));
+	if (err)
+		return err;
+
+	if (uptime)
+		*uptime = le32_to_cpu(reply[0]);
+
+	if (wakeup)
+		*wakeup = le32_to_cpu(reply[1]);
+
+	return 0;
+}
+
+static int omnia_read_time(struct device *dev, struct rtc_time *tm)
+{
+	u32 uptime;
+	int err;
+
+	err = omnia_get_uptime_wakeup(to_i2c_client(dev), &uptime, NULL);
+	if (err)
+		return err;
+
+	rtc_time64_to_tm(uptime, tm);
+
+	return 0;
+}
+
+static int omnia_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct omnia_mcu *mcu = i2c_get_clientdata(client);
+	u32 wakeup;
+	int err;
+
+	err = omnia_get_uptime_wakeup(client, NULL, &wakeup);
+	if (err)
+		return err;
+
+	alrm->enabled = !!wakeup;
+	rtc_time64_to_tm(wakeup ?: mcu->rtc_alarm, &alrm->time);
+
+	return 0;
+}
+
+static int omnia_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct omnia_mcu *mcu = i2c_get_clientdata(client);
+
+	mcu->rtc_alarm = rtc_tm_to_time64(&alrm->time);
+
+	if (alrm->enabled)
+		return omnia_cmd_write_u32(client, OMNIA_CMD_SET_WAKEUP,
+					   mcu->rtc_alarm);
+
+	return 0;
+}
+
+static int omnia_alarm_irq_enable(struct device *dev, unsigned int enabled)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct omnia_mcu *mcu = i2c_get_clientdata(client);
+
+	return omnia_cmd_write_u32(client, OMNIA_CMD_SET_WAKEUP,
+				   enabled ? mcu->rtc_alarm : 0);
+}
+
+static const struct rtc_class_ops omnia_rtc_ops = {
+	.read_time		= omnia_read_time,
+	.read_alarm		= omnia_read_alarm,
+	.set_alarm		= omnia_set_alarm,
+	.alarm_irq_enable	= omnia_alarm_irq_enable,
+};
+
+static int omnia_power_off(struct sys_off_data *data)
+{
+	struct omnia_mcu *mcu = data->cb_data;
+	__be32 tmp;
+	u8 cmd[9];
+	u16 arg;
+	int err;
+
+	if (mcu->front_button_poweron)
+		arg = OMNIA_CMD_POWER_OFF_POWERON_BUTTON;
+	else
+		arg = 0;
+
+	cmd[0] = OMNIA_CMD_POWER_OFF;
+	put_unaligned_le16(OMNIA_CMD_POWER_OFF_MAGIC, &cmd[1]);
+	put_unaligned_le16(arg, &cmd[3]);
+
+	/*
+	 * Although all values from and to MCU are passed in little-endian, the
+	 * MCU's CRC unit uses big-endian CRC32 polynomial (0x04c11db7), so we
+	 * need to use crc32_be() here.
+	 */
+	tmp = cpu_to_be32(get_unaligned_le32(&cmd[1]));
+	put_unaligned_le32(crc32_be(~0, (void *)&tmp, sizeof(tmp)), &cmd[5]);
+
+	err = omnia_cmd_write(mcu->client, cmd, sizeof(cmd));
+	if (err)
+		dev_err(&mcu->client->dev,
+			"Unable to send the poweroff command: %d\n", err);
+
+	return NOTIFY_DONE;
+}
+
+static int omnia_restart(struct sys_off_data *data)
+{
+	struct omnia_mcu *mcu = data->cb_data;
+	u8 cmd[3];
+	int err;
+
+	cmd[0] = OMNIA_CMD_GENERAL_CONTROL;
+
+	if (reboot_mode == REBOOT_HARD)
+		cmd[1] = cmd[2] = OMNIA_CTL_HARD_RST;
+	else
+		cmd[1] = cmd[2] = OMNIA_CTL_LIGHT_RST;
+
+	err = omnia_cmd_write(mcu->client, cmd, sizeof(cmd));
+	if (err)
+		dev_err(&mcu->client->dev,
+			"Unable to send the restart command: %d\n", err);
+
+	/*
+	 * MCU needs a little bit to process the I2C command, otherwise it will
+	 * do a light reset based on SOC SYSRES_OUT pin.
+	 */
+	mdelay(1);
+
+	return NOTIFY_DONE;
+}
+
+static ssize_t front_button_poweron_show(struct device *dev,
+					 struct device_attribute *a, char *buf)
+{
+	struct omnia_mcu *mcu = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%d\n", mcu->front_button_poweron);
+}
+
+static ssize_t front_button_poweron_store(struct device *dev,
+					  struct device_attribute *a,
+					  const char *buf, size_t count)
+{
+	struct omnia_mcu *mcu = dev_get_drvdata(dev);
+	bool val;
+	int err;
+
+	err = kstrtobool(buf, &val);
+	if (err)
+		return err;
+
+	mcu->front_button_poweron = val;
+
+	return count;
+}
+static DEVICE_ATTR_RW(front_button_poweron);
+
+static struct attribute *omnia_mcu_poweroff_attrs[] = {
+	&dev_attr_front_button_poweron.attr,
+	NULL
+};
+
+static umode_t poweroff_attrs_visible(struct kobject *kobj, struct attribute *a,
+				      int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct omnia_mcu *mcu = dev_get_drvdata(dev);
+
+	if (mcu->features & OMNIA_FEAT_POWEROFF_WAKEUP)
+		return a->mode;
+
+	return 0;
+}
+
+const struct attribute_group omnia_mcu_poweroff_group = {
+	.attrs = omnia_mcu_poweroff_attrs,
+	.is_visible = poweroff_attrs_visible,
+};
+
+int omnia_mcu_register_sys_off_and_wakeup(struct omnia_mcu *mcu)
+{
+	struct device *dev = &mcu->client->dev;
+	int err;
+
+	/* MCU restart is always available */
+	err = devm_register_sys_off_handler(dev, SYS_OFF_MODE_RESTART,
+					    SYS_OFF_PRIO_FIRMWARE,
+					    omnia_restart, mcu);
+	if (err)
+		return dev_err_probe(dev, err,
+				     "Cannot register system restart handler\n");
+
+	/*
+	 * Poweroff and wakeup are available only if POWEROFF_WAKEUP feature is
+	 * present.
+	 */
+	if (!(mcu->features & OMNIA_FEAT_POWEROFF_WAKEUP))
+		return 0;
+
+	err = devm_register_sys_off_handler(dev, SYS_OFF_MODE_POWER_OFF,
+					    SYS_OFF_PRIO_FIRMWARE,
+					    omnia_power_off, mcu);
+	if (err)
+		return dev_err_probe(dev, err,
+				     "Cannot register system power off handler\n");
+
+	mcu->rtcdev = devm_rtc_allocate_device(dev);
+	if (IS_ERR(mcu->rtcdev))
+		return dev_err_probe(dev, PTR_ERR(mcu->rtcdev),
+				     "Cannot allocate RTC device\n");
+
+	mcu->rtcdev->ops = &omnia_rtc_ops;
+	mcu->rtcdev->range_max = U32_MAX;
+	set_bit(RTC_FEATURE_ALARM_WAKEUP_ONLY, mcu->rtcdev->features);
+
+	err = devm_rtc_register_device(mcu->rtcdev);
+	if (err)
+		return dev_err_probe(dev, err, "Cannot register RTC device\n");
+
+	mcu->front_button_poweron = true;
+
+	return 0;
+}
diff --git a/drivers/platform/cznic/turris-omnia-mcu.h b/drivers/platform/cznic/turris-omnia-mcu.h
index 3d2dd0054499..5df421b03878 100644
--- a/drivers/platform/cznic/turris-omnia-mcu.h
+++ b/drivers/platform/cznic/turris-omnia-mcu.h
@@ -12,9 +12,11 @@
 #include <linux/gpio/driver.h>
 #include <linux/if_ether.h>
 #include <linux/mutex.h>
+#include <linux/rtc.h>
 #include <linux/types.h>
 #include <linux/workqueue.h>
 #include <asm/byteorder.h>
+#include <asm/unaligned.h>
 
 struct i2c_client;
 
@@ -36,6 +38,11 @@ struct omnia_mcu {
 	struct delayed_work button_release_emul_work;
 	unsigned long last_status;
 	bool button_pressed_emul;
+
+	/* RTC device for configuring wake-up */
+	struct rtc_device *rtcdev;
+	u32 rtc_alarm;
+	bool front_button_poweron;
 };
 
 int omnia_cmd_write_read(const struct i2c_client *client,
@@ -48,6 +55,17 @@ static inline int omnia_cmd_write(const struct i2c_client *client, void *cmd,
 	return omnia_cmd_write_read(client, cmd, len, NULL, 0);
 }
 
+static inline int omnia_cmd_write_u32(const struct i2c_client *client, u8 cmd,
+				      u32 val)
+{
+	u8 buf[5];
+
+	buf[0] = cmd;
+	put_unaligned_le32(val, &buf[1]);
+
+	return omnia_cmd_write(client, buf, sizeof(buf));
+}
+
 static inline int omnia_cmd_read(const struct i2c_client *client, u8 cmd,
 				 void *reply, unsigned int len)
 {
@@ -136,7 +154,9 @@ static inline int omnia_cmd_read_u8(const struct i2c_client *client, u8 cmd,
 }
 
 extern const struct attribute_group omnia_mcu_gpio_group;
+extern const struct attribute_group omnia_mcu_poweroff_group;
 
 int omnia_mcu_register_gpiochip(struct omnia_mcu *mcu);
+int omnia_mcu_register_sys_off_and_wakeup(struct omnia_mcu *mcu);
 
 #endif /* __TURRIS_OMNIA_MCU_H */
-- 
2.44.2

[PATCH v11 5/8] platform: cznic: turris-omnia-mcu: Add support for MCU watchdog

[Marek Behún]

Add support for the watchdog mechanism provided by the MCU.

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/platform/cznic/Kconfig                |   2 +
 drivers/platform/cznic/Makefile               |   1 +
 .../platform/cznic/turris-omnia-mcu-base.c    |   4 +
 .../cznic/turris-omnia-mcu-watchdog.c         | 128 ++++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |  24 ++++
 5 files changed, 159 insertions(+)
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-watchdog.c

diff --git a/drivers/platform/cznic/Kconfig b/drivers/platform/cznic/Kconfig
index c1e719235517..e262930b3faf 100644
--- a/drivers/platform/cznic/Kconfig
+++ b/drivers/platform/cznic/Kconfig
@@ -19,6 +19,7 @@ config TURRIS_OMNIA_MCU
 	select GPIOLIB
 	select GPIOLIB_IRQCHIP
 	select RTC_CLASS
+	select WATCHDOG_CORE
 	help
 	  Say Y here to add support for the features implemented by the
 	  microcontroller on the CZ.NIC's Turris Omnia SOHO router.
@@ -26,6 +27,7 @@ config TURRIS_OMNIA_MCU
 	  - board poweroff into true low power mode (with voltage regulators
 	    disabled) and the ability to configure wake up from this mode (via
 	    rtcwake)
+	  - MCU watchdog
 	  - GPIO pins
 	    - to get front button press events (the front button can be
 	      configured either to generate press events to the CPU or to change
diff --git a/drivers/platform/cznic/Makefile b/drivers/platform/cznic/Makefile
index a185ef882e44..687f7718c0a1 100644
--- a/drivers/platform/cznic/Makefile
+++ b/drivers/platform/cznic/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_TURRIS_OMNIA_MCU)	+= turris-omnia-mcu.o
 turris-omnia-mcu-y		:= turris-omnia-mcu-base.o
 turris-omnia-mcu-y		+= turris-omnia-mcu-gpio.o
 turris-omnia-mcu-y		+= turris-omnia-mcu-sys-off-wakeup.o
+turris-omnia-mcu-y		+= turris-omnia-mcu-watchdog.o
diff --git a/drivers/platform/cznic/turris-omnia-mcu-base.c b/drivers/platform/cznic/turris-omnia-mcu-base.c
index 479f355f730a..f44996588d38 100644
--- a/drivers/platform/cznic/turris-omnia-mcu-base.c
+++ b/drivers/platform/cznic/turris-omnia-mcu-base.c
@@ -376,6 +376,10 @@ static int omnia_mcu_probe(struct i2c_client *client)
 	if (err)
 		return err;
 
+	err = omnia_mcu_register_watchdog(mcu);
+	if (err)
+		return err;
+
 	return omnia_mcu_register_gpiochip(mcu);
 }
 
diff --git a/drivers/platform/cznic/turris-omnia-mcu-watchdog.c b/drivers/platform/cznic/turris-omnia-mcu-watchdog.c
new file mode 100644
index 000000000000..da2efc32df8f
--- /dev/null
+++ b/drivers/platform/cznic/turris-omnia-mcu-watchdog.c
@@ -0,0 +1,128 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CZ.NIC's Turris Omnia MCU watchdog driver
+ *
+ * 2024 by Marek Behún <kabel@kernel.org>
+ */
+
+#include <linux/i2c.h>
+#include <linux/moduleparam.h>
+#include <linux/turris-omnia-mcu-interface.h>
+#include <linux/types.h>
+#include <linux/units.h>
+#include <linux/watchdog.h>
+
+#include "turris-omnia-mcu.h"
+
+#define WATCHDOG_TIMEOUT		120
+
+static unsigned int timeout;
+module_param(timeout, int, 0);
+MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds");
+
+static bool nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, bool, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
+			   __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+static int omnia_wdt_start(struct watchdog_device *wdt)
+{
+	struct omnia_mcu *mcu = watchdog_get_drvdata(wdt);
+
+	return omnia_cmd_write_u8(mcu->client, OMNIA_CMD_SET_WATCHDOG_STATE, 1);
+}
+
+static int omnia_wdt_stop(struct watchdog_device *wdt)
+{
+	struct omnia_mcu *mcu = watchdog_get_drvdata(wdt);
+
+	return omnia_cmd_write_u8(mcu->client, OMNIA_CMD_SET_WATCHDOG_STATE, 0);
+}
+
+static int omnia_wdt_ping(struct watchdog_device *wdt)
+{
+	struct omnia_mcu *mcu = watchdog_get_drvdata(wdt);
+
+	return omnia_cmd_write_u8(mcu->client, OMNIA_CMD_SET_WATCHDOG_STATE, 1);
+}
+
+static int omnia_wdt_set_timeout(struct watchdog_device *wdt,
+				 unsigned int timeout)
+{
+	struct omnia_mcu *mcu = watchdog_get_drvdata(wdt);
+
+	return omnia_cmd_write_u16(mcu->client, OMNIA_CMD_SET_WDT_TIMEOUT,
+				   timeout * DECI);
+}
+
+static unsigned int omnia_wdt_get_timeleft(struct watchdog_device *wdt)
+{
+	struct omnia_mcu *mcu = watchdog_get_drvdata(wdt);
+	u16 timeleft;
+	int err;
+
+	err = omnia_cmd_read_u16(mcu->client, OMNIA_CMD_GET_WDT_TIMELEFT,
+				 &timeleft);
+	if (err) {
+		dev_err(&mcu->client->dev, "Cannot get watchdog timeleft: %d\n",
+			err);
+		return 0;
+	}
+
+	return timeleft / DECI;
+}
+
+static const struct watchdog_info omnia_wdt_info = {
+	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+	.identity = "Turris Omnia MCU Watchdog",
+};
+
+static const struct watchdog_ops omnia_wdt_ops = {
+	.owner		= THIS_MODULE,
+	.start		= omnia_wdt_start,
+	.stop		= omnia_wdt_stop,
+	.ping		= omnia_wdt_ping,
+	.set_timeout	= omnia_wdt_set_timeout,
+	.get_timeleft	= omnia_wdt_get_timeleft,
+};
+
+int omnia_mcu_register_watchdog(struct omnia_mcu *mcu)
+{
+	struct device *dev = &mcu->client->dev;
+	u8 state;
+	int err;
+
+	if (!(mcu->features & OMNIA_FEAT_WDT_PING))
+		return 0;
+
+	mcu->wdt.info = &omnia_wdt_info;
+	mcu->wdt.ops = &omnia_wdt_ops;
+	mcu->wdt.parent = dev;
+	mcu->wdt.min_timeout = 1;
+	mcu->wdt.max_timeout = 65535 / DECI;
+
+	mcu->wdt.timeout = WATCHDOG_TIMEOUT;
+	watchdog_init_timeout(&mcu->wdt, timeout, dev);
+
+	watchdog_set_drvdata(&mcu->wdt, mcu);
+
+	omnia_wdt_set_timeout(&mcu->wdt, mcu->wdt.timeout);
+
+	err = omnia_cmd_read_u8(mcu->client, OMNIA_CMD_GET_WATCHDOG_STATE,
+				&state);
+	if (err)
+		return dev_err_probe(dev, err,
+				     "Cannot get MCU watchdog state\n");
+
+	if (state)
+		set_bit(WDOG_HW_RUNNING, &mcu->wdt.status);
+
+	watchdog_set_nowayout(&mcu->wdt, nowayout);
+	watchdog_stop_on_reboot(&mcu->wdt);
+	err = devm_watchdog_register_device(dev, &mcu->wdt);
+	if (err)
+		return dev_err_probe(dev, err,
+				     "Cannot register MCU watchdog\n");
+
+	return 0;
+}
diff --git a/drivers/platform/cznic/turris-omnia-mcu.h b/drivers/platform/cznic/turris-omnia-mcu.h
index 5df421b03878..5f846909934f 100644
--- a/drivers/platform/cznic/turris-omnia-mcu.h
+++ b/drivers/platform/cznic/turris-omnia-mcu.h
@@ -14,6 +14,7 @@
 #include <linux/mutex.h>
 #include <linux/rtc.h>
 #include <linux/types.h>
+#include <linux/watchdog.h>
 #include <linux/workqueue.h>
 #include <asm/byteorder.h>
 #include <asm/unaligned.h>
@@ -43,6 +44,9 @@ struct omnia_mcu {
 	struct rtc_device *rtcdev;
 	u32 rtc_alarm;
 	bool front_button_poweron;
+
+	/* MCU watchdog */
+	struct watchdog_device wdt;
 };
 
 int omnia_cmd_write_read(const struct i2c_client *client,
@@ -55,6 +59,25 @@ static inline int omnia_cmd_write(const struct i2c_client *client, void *cmd,
 	return omnia_cmd_write_read(client, cmd, len, NULL, 0);
 }
 
+static inline int omnia_cmd_write_u8(const struct i2c_client *client, u8 cmd,
+				     u8 val)
+{
+	u8 buf[2] = { cmd, val };
+
+	return omnia_cmd_write(client, buf, sizeof(buf));
+}
+
+static inline int omnia_cmd_write_u16(const struct i2c_client *client, u8 cmd,
+				      u16 val)
+{
+	u8 buf[3];
+
+	buf[0] = cmd;
+	put_unaligned_le16(val, &buf[1]);
+
+	return omnia_cmd_write(client, buf, sizeof(buf));
+}
+
 static inline int omnia_cmd_write_u32(const struct i2c_client *client, u8 cmd,
 				      u32 val)
 {
@@ -158,5 +181,6 @@ extern const struct attribute_group omnia_mcu_poweroff_group;
 
 int omnia_mcu_register_gpiochip(struct omnia_mcu *mcu);
 int omnia_mcu_register_sys_off_and_wakeup(struct omnia_mcu *mcu);
+int omnia_mcu_register_watchdog(struct omnia_mcu *mcu);
 
 #endif /* __TURRIS_OMNIA_MCU_H */
-- 
2.44.2

[PATCH v11 6/8] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG

[Marek Behún]

Add support for true random number generator provided by the MCU.
New Omnia boards come without the Atmel SHA204-A chip. Instead the
crypto functionality is provided by new microcontroller, which has
a TRNG peripheral.

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/platform/cznic/Kconfig                |   2 +
 drivers/platform/cznic/Makefile               |   1 +
 .../platform/cznic/turris-omnia-mcu-base.c    |   6 +-
 .../platform/cznic/turris-omnia-mcu-gpio.c    |   2 +-
 .../platform/cznic/turris-omnia-mcu-trng.c    | 103 ++++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |   8 ++
 6 files changed, 120 insertions(+), 2 deletions(-)
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-trng.c

diff --git a/drivers/platform/cznic/Kconfig b/drivers/platform/cznic/Kconfig
index e262930b3faf..6edac80d5fa3 100644
--- a/drivers/platform/cznic/Kconfig
+++ b/drivers/platform/cznic/Kconfig
@@ -18,6 +18,7 @@ config TURRIS_OMNIA_MCU
 	depends on I2C
 	select GPIOLIB
 	select GPIOLIB_IRQCHIP
+	select HW_RANDOM
 	select RTC_CLASS
 	select WATCHDOG_CORE
 	help
@@ -27,6 +28,7 @@ config TURRIS_OMNIA_MCU
 	  - board poweroff into true low power mode (with voltage regulators
 	    disabled) and the ability to configure wake up from this mode (via
 	    rtcwake)
+	  - true random number generator (if available on the MCU)
 	  - MCU watchdog
 	  - GPIO pins
 	    - to get front button press events (the front button can be
diff --git a/drivers/platform/cznic/Makefile b/drivers/platform/cznic/Makefile
index 687f7718c0a1..eae4c6b341ff 100644
--- a/drivers/platform/cznic/Makefile
+++ b/drivers/platform/cznic/Makefile
@@ -4,4 +4,5 @@ obj-$(CONFIG_TURRIS_OMNIA_MCU)	+= turris-omnia-mcu.o
 turris-omnia-mcu-y		:= turris-omnia-mcu-base.o
 turris-omnia-mcu-y		+= turris-omnia-mcu-gpio.o
 turris-omnia-mcu-y		+= turris-omnia-mcu-sys-off-wakeup.o
+turris-omnia-mcu-y		+= turris-omnia-mcu-trng.o
 turris-omnia-mcu-y		+= turris-omnia-mcu-watchdog.o
diff --git a/drivers/platform/cznic/turris-omnia-mcu-base.c b/drivers/platform/cznic/turris-omnia-mcu-base.c
index f44996588d38..1d4153a96526 100644
--- a/drivers/platform/cznic/turris-omnia-mcu-base.c
+++ b/drivers/platform/cznic/turris-omnia-mcu-base.c
@@ -380,7 +380,11 @@ static int omnia_mcu_probe(struct i2c_client *client)
 	if (err)
 		return err;
 
-	return omnia_mcu_register_gpiochip(mcu);
+	err = omnia_mcu_register_gpiochip(mcu);
+	if (err)
+		return err;
+
+	return omnia_mcu_register_trng(mcu);
 }
 
 static const struct of_device_id of_omnia_mcu_match[] = {
diff --git a/drivers/platform/cznic/turris-omnia-mcu-gpio.c b/drivers/platform/cznic/turris-omnia-mcu-gpio.c
index bc7965e6c879..972364d3d223 100644
--- a/drivers/platform/cznic/turris-omnia-mcu-gpio.c
+++ b/drivers/platform/cznic/turris-omnia-mcu-gpio.c
@@ -174,7 +174,7 @@ static const struct omnia_gpio omnia_gpios[64] = {
 };
 
 /* mapping from interrupts to indexes of GPIOs in the omnia_gpios array */
-static const u8 omnia_int_to_gpio_idx[32] = {
+const u8 omnia_int_to_gpio_idx[32] = {
 	[__bf_shf(OMNIA_INT_CARD_DET)]			= 4,
 	[__bf_shf(OMNIA_INT_MSATA_IND)]			= 5,
 	[__bf_shf(OMNIA_INT_USB30_OVC)]			= 6,
diff --git a/drivers/platform/cznic/turris-omnia-mcu-trng.c b/drivers/platform/cznic/turris-omnia-mcu-trng.c
new file mode 100644
index 000000000000..fbde00f3fca1
--- /dev/null
+++ b/drivers/platform/cznic/turris-omnia-mcu-trng.c
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CZ.NIC's Turris Omnia MCU TRNG driver
+ *
+ * 2024 by Marek Behún <kabel@kernel.org>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/completion.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+#include <linux/hw_random.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/minmax.h>
+#include <linux/module.h>
+#include <linux/string.h>
+#include <linux/turris-omnia-mcu-interface.h>
+#include <linux/types.h>
+
+#include "turris-omnia-mcu.h"
+
+#define OMNIA_CMD_TRNG_MAX_ENTROPY_LEN	64
+
+static irqreturn_t omnia_trng_irq_handler(int irq, void *dev_id)
+{
+	struct omnia_mcu *mcu = dev_id;
+
+	complete(&mcu->trng_completion);
+
+	return IRQ_HANDLED;
+}
+
+static int omnia_trng_read(struct hwrng *rng, void *data, size_t max, bool wait)
+{
+	struct omnia_mcu *mcu = (struct omnia_mcu *)rng->priv;
+	u8 reply[1 + OMNIA_CMD_TRNG_MAX_ENTROPY_LEN];
+	int err, bytes;
+
+	if (!wait && !completion_done(&mcu->trng_completion))
+		return 0;
+
+	do {
+		if (wait_for_completion_interruptible(&mcu->trng_completion))
+			return -ERESTARTSYS;
+
+		err = omnia_cmd_read(mcu->client,
+				     OMNIA_CMD_TRNG_COLLECT_ENTROPY,
+				     reply, sizeof(reply));
+		if (err)
+			return err;
+
+		bytes = min3(reply[0], max, OMNIA_CMD_TRNG_MAX_ENTROPY_LEN);
+	} while (wait && !bytes);
+
+	memcpy(data, &reply[1], bytes);
+
+	return bytes;
+}
+
+int omnia_mcu_register_trng(struct omnia_mcu *mcu)
+{
+	struct device *dev = &mcu->client->dev;
+	u8 irq_idx, dummy;
+	int irq, err;
+
+	if (!(mcu->features & OMNIA_FEAT_TRNG))
+		return 0;
+
+	irq_idx = omnia_int_to_gpio_idx[__bf_shf(OMNIA_INT_TRNG)];
+	irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
+	if (irq < 0)
+		return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");
+
+	/*
+	 * If someone else cleared the TRNG interrupt but did not read the
+	 * entropy, a new interrupt won't be generated, and entropy collection
+	 * will be stuck. Ensure an interrupt will be generated by executing
+	 * the collect entropy command (and discarding the result).
+	 */
+	err = omnia_cmd_read(mcu->client, OMNIA_CMD_TRNG_COLLECT_ENTROPY,
+			     &dummy, 1);
+	if (err)
+		return err;
+
+	init_completion(&mcu->trng_completion);
+
+	err = devm_request_threaded_irq(dev, irq, NULL, omnia_trng_irq_handler,
+					IRQF_ONESHOT, "turris-omnia-mcu-trng",
+					mcu);
+	if (err)
+		return dev_err_probe(dev, err, "Cannot request TRNG IRQ\n");
+
+	mcu->trng.name = "turris-omnia-mcu-trng";
+	mcu->trng.read = omnia_trng_read;
+	mcu->trng.priv = (unsigned long)mcu;
+
+	err = devm_hwrng_register(dev, &mcu->trng);
+	if (err)
+		return dev_err_probe(dev, err, "Cannot register TRNG\n");
+
+	return 0;
+}
diff --git a/drivers/platform/cznic/turris-omnia-mcu.h b/drivers/platform/cznic/turris-omnia-mcu.h
index 5f846909934f..ee8d58516156 100644
--- a/drivers/platform/cznic/turris-omnia-mcu.h
+++ b/drivers/platform/cznic/turris-omnia-mcu.h
@@ -9,7 +9,9 @@
 #define __TURRIS_OMNIA_MCU_H
 
 #include <linux/bitops.h>
+#include <linux/completion.h>
 #include <linux/gpio/driver.h>
+#include <linux/hw_random.h>
 #include <linux/if_ether.h>
 #include <linux/mutex.h>
 #include <linux/rtc.h>
@@ -47,6 +49,10 @@ struct omnia_mcu {
 
 	/* MCU watchdog */
 	struct watchdog_device wdt;
+
+	/* true random number generator */
+	struct hwrng trng;
+	struct completion trng_completion;
 };
 
 int omnia_cmd_write_read(const struct i2c_client *client,
@@ -176,11 +182,13 @@ static inline int omnia_cmd_read_u8(const struct i2c_client *client, u8 cmd,
 	return omnia_cmd_read(client, cmd, reply, sizeof(*reply));
 }
 
+extern const u8 omnia_int_to_gpio_idx[32];
 extern const struct attribute_group omnia_mcu_gpio_group;
 extern const struct attribute_group omnia_mcu_poweroff_group;
 
 int omnia_mcu_register_gpiochip(struct omnia_mcu *mcu);
 int omnia_mcu_register_sys_off_and_wakeup(struct omnia_mcu *mcu);
+int omnia_mcu_register_trng(struct omnia_mcu *mcu);
 int omnia_mcu_register_watchdog(struct omnia_mcu *mcu);
 
 #endif /* __TURRIS_OMNIA_MCU_H */
-- 
2.44.2

[PATCH v11 7/8] ARM: dts: turris-omnia: Add MCU system-controller node

[Marek Behún]

Turris Omnia's MCU provides various features that can be configured over
I2C at address 0x2a. Add device-tree node.

This does not carry a Fixes tag - we do not want this to get backported
to stable kernels for the following reason: U-Boot since v2022.10
inserts a phy-reset-gpio property into the WAN ethernet node pointing to
the MCU node if it finds the MCU node with a cznic,turris-omnia-mcu
compatible. Thus if this change got backported to a stable kernel, the
WAN interface driver would defer probe indefinitely (since it would wait
for the turris-omnia-mcu driver which would not be present).

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 .../dts/marvell/armada-385-turris-omnia.dts   | 22 ++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts b/arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts
index 7b755bb4e4e7..59079d63fe27 100644
--- a/arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts
+++ b/arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts
@@ -218,7 +218,22 @@ i2c@0 {
 			#size-cells = <0>;
 			reg = <0>;
 
-			/* STM32F0 command interface at address 0x2a */
+			mcu: system-controller@2a {
+				compatible = "cznic,turris-omnia-mcu";
+				reg = <0x2a>;
+
+				pinctrl-names = "default";
+				pinctrl-0 = <&mcu_pins>;
+
+				interrupt-parent = <&gpio1>;
+				interrupts = <11 IRQ_TYPE_NONE>;
+
+				gpio-controller;
+				#gpio-cells = <3>;
+
+				interrupt-controller;
+				#interrupt-cells = <2>;
+			};
 
 			led-controller@2b {
 				compatible = "cznic,turris-omnia-leds";
@@ -501,6 +516,11 @@ fixed-link {
 };
 
 &pinctrl {
+	mcu_pins: mcu-pins {
+		marvell,pins = "mpp43";
+		marvell,function = "gpio";
+	};
+
 	pcawan_pins: pcawan-pins {
 		marvell,pins = "mpp46";
 		marvell,function = "gpio";
-- 
2.44.2

[PATCH v11 8/8] ARM: dts: turris-omnia: Add GPIO key node for front button

[Marek Behún]

Now that we have the MCU device-tree node, which acts as a GPIO
controller, add GPIO key node for the front button.

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 .../boot/dts/marvell/armada-385-turris-omnia.dts    | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts b/arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts
index 59079d63fe27..43202890c959 100644
--- a/arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts
+++ b/arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts
@@ -112,6 +112,19 @@ sfp: sfp {
 		status = "disabled";
 	};
 
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		front-button {
+			label = "Front Button";
+			linux,code = <KEY_VENDOR>;
+			linux,can-disable;
+			gpios = <&mcu 0 12 GPIO_ACTIVE_HIGH>;
+			/* debouncing is done by the microcontroller */
+			debounce-interval = <0>;
+		};
+	};
+
 	sound {
 		compatible = "simple-audio-card";
 		simple-audio-card,name = "SPDIF";
-- 
2.44.2

Re: [PATCH v11 2/8] platform: cznic: Add preliminary support for Turris Omnia MCU

[Andy Shevchenko]

On Wed, Jun 5, 2024 at 7:19 PM Marek Behún <kabel@kernel.org> wrote:
>
> Add the basic skeleton for a new platform driver for the microcontroller
> found on the Turris Omnia board.

...

I paid attention to this block because of ETH_ALEN, see below.

> +#include <linux/array_size.h>
> +#include <linux/bits.h>
> +#include <linux/device.h>
> +#include <linux/hex.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>

> +#include <linux/turris-omnia-mcu-interface.h>

This is part of the niche of the driver, I would move it

> +#include <linux/types.h>

t is followed by s :-)

> +#include <linux/string.h>
> +#include <linux/sysfs.h>

...here as a separate group.

> +#include "turris-omnia-mcu.h"

...

> +       /* we can't use ether_addr_copy() because reply is not u16-aligned */
> +       memcpy(mcu->board_first_mac, &reply[9], ETH_ALEN);

The inclusion block misses the header for ETH_ALEN, but I realise that
instead it's better to use sizeof() as it makes this rely to the real
size of the buffer and header is not needed either.

...

Other than above LGTM, FWIW,
Reviewed-by: Andy Shevchenko <andy@kernel.org>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v11 3/8] platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs

[Andy Shevchenko]

On Wed, Jun 5, 2024 at 7:19 PM Marek Behún <kabel@kernel.org> wrote:
>
> Add support for GPIOs connected to the MCU on the Turris Omnia board.
>
> This includes:
> - front button pin
> - enable pins for USB regulators
> - MiniPCIe / mSATA card presence pins in MiniPCIe port 0
> - LED output pins from WAN ethernet PHY, LAN switch and MiniPCIe ports
> - on board revisions 32+ also various peripheral resets and another
>   voltage regulator enable pin

...

> +#include <linux/array_size.h>
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/bug.h>
> +#include <linux/cleanup.h>
> +#include <linux/device.h>
> +#include <linux/devm-helpers.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/mutex.h>
> +#include <linux/sysfs.h>

> +#include <linux/turris-omnia-mcu-interface.h>

As per previous patch.

> +#include <linux/types.h>
> +#include <linux/workqueue.h>
> +#include <asm/unaligned.h>
> +
> +#include "turris-omnia-mcu.h"

...

> +struct omnia_gpio {
> +       u8 cmd, ctl_cmd, bit, ctl_bit, int_bit;
> +       bool has_int;
> +       u16 feat, feat_mask;
> +};

This is hard to follow, please make one field per line.

...

> +#define _DEF_GPIO(_cmd, _ctl_cmd, _bit, _ctl_bit, _int_bit, _feat, _feat_mask) \
> +       {                                                               \
> +               .cmd = _cmd,                                            \
> +               .ctl_cmd = _ctl_cmd,                                    \
> +               .bit = _bit,                                            \
> +               .ctl_bit = _ctl_bit,                                    \
> +               .int_bit = _int_bit,                                    \

> +               .has_int = (_int_bit) != -1,                            \

Useless field, you can always perform this check in the code Since you
have int_bit field defined. Filling it with kinda garbage is not an
excuse, you can just define the INVALID_IRQ like many drivers do that
want to use unsigned types.

#define OMNIA_GPIO_INVALID_INT_BIT  0xff

Possibly to have
static inline is_int_bit_valid(struct omnia_gpio *gpio)
{
  return gpio->int_bit != OMNIA_GPIO_INVALID_INT_BIT;
}

...

> +       /*
> +        * If firmware does support the new interrupt API, we may have cached
> +        * the value of a GPIO in the interrupt service routine. If not, read
> +        * the relevant bit now.
> +        */

> +       if (gpio->has_int && test_bit(gpio->int_bit, &mcu->is_cached))

See above.

> +               return test_bit(gpio->int_bit, &mcu->cached);
> +
> +       return omnia_cmd_read_bit(mcu->client, gpio->cmd, BIT(gpio->bit));
> +}


> +       /* assign relevant bits in result */
> +       for_each_set_bit(i, mask, ARRAY_SIZE(omnia_gpios)) {
> +               field = _relevant_field_for_sts_cmd(omnia_gpios[i].cmd,
> +                                                   &sts, &ext_sts, &ext_ctl);

> +               if (field)

Perhaps

  if (!field)
    continue;

?

> +                       __assign_bit(i, bits,
> +                                    test_bit(omnia_gpios[i].bit, field));

This will become one line after the above change.

> +       }
> +
> +       return 0;
> +}

...

> +       int i;

Why signed?

> +       for_each_set_bit(i, mask, ARRAY_SIZE(omnia_gpios)) {
> +               unsigned long *field, *field_mask;
> +               u8 bit = omnia_gpios[i].ctl_bit;
> +
> +               switch (omnia_gpios[i].ctl_cmd) {
> +               case OMNIA_CMD_GENERAL_CONTROL:
> +                       field = &ctl;
> +                       field_mask = &ctl_mask;
> +                       break;
> +               case OMNIA_CMD_EXT_CONTROL:
> +                       field = &ext_ctl;
> +                       field_mask = &ext_ctl_mask;
> +                       break;
> +               default:
> +                       field = field_mask = NULL;
> +                       break;
> +               }

> +               if (field) {

Like the above

  if (!field)
    continue;

> +                       __set_bit(bit, field_mask);
> +                       __assign_bit(bit, field, test_bit(i, bits));
> +               }
> +       }

...

> +       if (!(type & IRQ_TYPE_EDGE_BOTH)) {
> +               dev_err(dev, "irq %u: unsupported type %u\n", d->irq, type);

You have hwirq, no need to dereference again.

> +               return -EINVAL;
> +       }

...

> +/**
> + * omnia_mask_interleave - Interleaves the bytes from @rising and @falling
> + *     @dst: the destination u8 array of interleaved bytes
> + *     @rising: rising mask
> + *     @falling: falling mask

Why so many spaces before @? One is enough.

> + *
> + * Interleaves the little-endian bytes from @rising and @falling words.
> + *
> + * If @rising = (r0, r1, r2, r3) and @falling = (f0, f1, f2, f3), the result is
> + * @dst = (r0, f0, r1, f1, r2, f2, r3, f3).
> + *
> + * The MCU receives an interrupt mask and reports a pending interrupt bitmap in
> + * this interleaved format. The rationale behind this is that the low-indexed
> + * bits are more important - in many cases, the user will be interested only in
> + * interrupts with indexes 0 to 7, and so the system can stop reading after
> + * first 2 bytes (r0, f0), to save time on the slow I2C bus.
> + *
> + * Feel free to remove this function and its inverse, omnia_mask_deinterleave,
> + * and use an appropriate bitmap_*() function once such a function exists.
> + */
> +static void
> +omnia_mask_interleave(u8 *dst, unsigned long rising, unsigned long falling)

But rising and failing should be either u64 or unsigned long *.

> +{
> +       for (int i = 0; i < sizeof(u32); ++i) {

In other cases you use:
1) unsigned
2) post-increment

What makes this one special?

> +               dst[2 * i] = rising >> (8 * i);
> +               dst[2 * i + 1] = falling >> (8 * i);
> +       }
> +}

...

> +/**
> + * omnia_mask_deinterleave - Deinterleaves the bytes into @rising and @falling
> + *     @src: the source u8 array containing the interleaved bytes
> + *     @rising: pointer where to store the rising mask gathered from @src
> + *     @falling: pointer where to store the falling mask gathered from @src
> + *
> + * This is the inverse function to omnia_mask_interleave.
> + */
> +static void omnia_mask_deinterleave(const u8 *src, unsigned long *rising,
> +                                   unsigned long *falling)
> +{
> +       *rising = *falling = 0;
> +
> +       for (int i = 0; i < sizeof(u32); ++i) {
> +               *rising |= src[2 * i] << (8 * i);
> +               *falling |= src[2 * i + 1] << (8 * i);
> +       }
> +}

Same comments as per above function.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v11 4/8] platform: cznic: turris-omnia-mcu: Add support for poweroff and wakeup

[Andy Shevchenko]

On Wed, Jun 5, 2024 at 7:19 PM Marek Behún <kabel@kernel.org> wrote:
>
> Add support for true board poweroff (MCU can disable all unnecessary
> voltage regulators) and wakeup at a specified time, implemented via a
> RTC driver so that the rtcwake utility can be used to configure it.

...

> +#include <linux/crc32.h>
> +#include <linux/delay.h>

+ device.h
+ err.h

> +#include <linux/i2c.h>

+ kstrtox.h

> +#include <linux/reboot.h>
> +#include <linux/rtc.h>
> +#include <linux/sysfs.h>

> +#include <linux/turris-omnia-mcu-interface.h>

As per other patches.

> +#include <linux/types.h>
> +
> +#include "turris-omnia-mcu.h"

...

> +       struct device *dev = kobj_to_dev(kobj);

Is this declared in device.h or elsewhere?

...

> +#include <linux/rtc.h>

I don't see you need it.
Just

struct rtc_device;

should suffice.

>  #include <linux/types.h>
>  #include <linux/workqueue.h>
>  #include <asm/byteorder.h>
> +#include <asm/unaligned.h>

...

With the above being addressed, LGTM, FWIW,
Reviewed-by: Andy Shevchenko <andy@kernel.org>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v11 5/8] platform: cznic: turris-omnia-mcu: Add support for MCU watchdog

[Andy Shevchenko]

On Wed, Jun 5, 2024 at 7:19 PM Marek Behún <kabel@kernel.org> wrote:
>
> Add support for the watchdog mechanism provided by the MCU.

...

+ bitops.h
+ dev_printk.h

> +#include <linux/i2c.h>
> +#include <linux/moduleparam.h>

> +#include <linux/turris-omnia-mcu-interface.h>

As per other patches.

> +#include <linux/types.h>
> +#include <linux/units.h>
> +#include <linux/watchdog.h>
> +
> +#include "turris-omnia-mcu.h"

struct device;

Alternatively instead of dev_printk.h + struct device forward
declaration --> device.h.

...

With the above being addressed, LGTM, FWIW,
Reviewed-by: Andy Shevchenko <andy@kernel.org>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v11 6/8] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG

[Andy Shevchenko]

On Wed, Jun 5, 2024 at 7:19 PM Marek Behún <kabel@kernel.org> wrote:
>
> Add support for true random number generator provided by the MCU.
> New Omnia boards come without the Atmel SHA204-A chip. Instead the
> crypto functionality is provided by new microcontroller, which has
> a TRNG peripheral.

+Cc: Bart for gpiochip_get_desc() usage.

...

> +#include <linux/bitfield.h>
> +#include <linux/completion.h>

+ errno.h

> +#include <linux/gpio/consumer.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/hw_random.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/minmax.h>
> +#include <linux/module.h>
> +#include <linux/string.h>

> +#include <linux/turris-omnia-mcu-interface.h>

As per other patches.

> +#include <linux/types.h>
> +
> +#include "turris-omnia-mcu.h"

...

> +       irq_idx = omnia_int_to_gpio_idx[__bf_shf(OMNIA_INT_TRNG)];
> +       irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
> +       if (irq < 0)
> +               return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");

Okay, it's a bit more complicated than that. The gpiochip_get_desc()
shouldn't be used. Bart, what can you suggest to do here? Opencoding
it doesn't sound to me a (fully) correct approach in a long term.

--
With Best Regards,
Andy Shevchenko

Re: [PATCH v11 3/8] platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs

[Andy Shevchenko]

On Wed, Jun 5, 2024 at 9:29 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Jun 5, 2024 at 7:19 PM Marek Behún <kabel@kernel.org> wrote:

...

> > +/**
> > + * omnia_mask_interleave - Interleaves the bytes from @rising and @falling
> > + *     @dst: the destination u8 array of interleaved bytes
> > + *     @rising: rising mask
> > + *     @falling: falling mask
>
> Why so many spaces before @? One is enough.
>
> > + *
> > + * Interleaves the little-endian bytes from @rising and @falling words.
> > + *
> > + * If @rising = (r0, r1, r2, r3) and @falling = (f0, f1, f2, f3), the result is
> > + * @dst = (r0, f0, r1, f1, r2, f2, r3, f3).
> > + *
> > + * The MCU receives an interrupt mask and reports a pending interrupt bitmap in
> > + * this interleaved format. The rationale behind this is that the low-indexed
> > + * bits are more important - in many cases, the user will be interested only in
> > + * interrupts with indexes 0 to 7, and so the system can stop reading after
> > + * first 2 bytes (r0, f0), to save time on the slow I2C bus.
> > + *
> > + * Feel free to remove this function and its inverse, omnia_mask_deinterleave,
> > + * and use an appropriate bitmap_*() function once such a function exists.
> > + */
> > +static void
> > +omnia_mask_interleave(u8 *dst, unsigned long rising, unsigned long falling)

> But rising and failing should be either u64 or unsigned long *.

Ah, sorry, I misread the use, discard this single comment.

> > +{
> > +       for (int i = 0; i < sizeof(u32); ++i) {
>
> In other cases you use:
> 1) unsigned
> 2) post-increment
>
> What makes this one special?
>
> > +               dst[2 * i] = rising >> (8 * i);
> > +               dst[2 * i + 1] = falling >> (8 * i);
> > +       }
> > +}
>
> ...
>
> > +/**
> > + * omnia_mask_deinterleave - Deinterleaves the bytes into @rising and @falling
> > + *     @src: the source u8 array containing the interleaved bytes
> > + *     @rising: pointer where to store the rising mask gathered from @src
> > + *     @falling: pointer where to store the falling mask gathered from @src
> > + *
> > + * This is the inverse function to omnia_mask_interleave.
> > + */
> > +static void omnia_mask_deinterleave(const u8 *src, unsigned long *rising,
> > +                                   unsigned long *falling)
> > +{
> > +       *rising = *falling = 0;
> > +
> > +       for (int i = 0; i < sizeof(u32); ++i) {
> > +               *rising |= src[2 * i] << (8 * i);
> > +               *falling |= src[2 * i + 1] << (8 * i);
> > +       }
> > +}
>
> Same comments as per above function.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v11 0/8] Turris Omnia MCU driver

[Andy Shevchenko]

On Wed, Jun 5, 2024 at 7:19 PM Marek Behún <kabel@kernel.org> wrote:
>
> Hello Andy, Hans, Ilpo, Arnd, Gregory, and others,
>
> this is v11 of the series adding Turris Omnia MCU driver.

Thank you!
There are a few small issues here and there, but overall LGTM. The
only one main question is what to do with gpiochip_get_desc(). I Cc'ed
Bart for this.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v11 0/8] Turris Omnia MCU driver

[Marek Behún]

On Wed, 5 Jun 2024 22:05:37 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Wed, Jun 5, 2024 at 7:19 PM Marek Behún <kabel@kernel.org> wrote:
> >
> > Hello Andy, Hans, Ilpo, Arnd, Gregory, and others,
> >
> > this is v11 of the series adding Turris Omnia MCU driver.  
> 
> Thank you!
> There are a few small issues here and there, but overall LGTM. The
> only one main question is what to do with gpiochip_get_desc(). I Cc'ed
> Bart for this.

Thank you for the review, I am going to apply the changes you requested
and wait for Bart, and we'll see what to do with the
gpiochip_get_desc().

Marek

Re: [PATCH v11 4/8] platform: cznic: turris-omnia-mcu: Add support for poweroff and wakeup

[Marek Behún]

On Wed, 5 Jun 2024 21:41:04 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> > +       struct device *dev = kobj_to_dev(kobj);  
> 
> Is this declared in device.h or elsewhere?

It is in device.h.

Re: [PATCH v11 6/8] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG

[Marek Behún]

On Wed, 5 Jun 2024 22:00:20 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> > +       irq_idx = omnia_int_to_gpio_idx[__bf_shf(OMNIA_INT_TRNG)];
> > +       irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
> > +       if (irq < 0)
> > +               return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");  
> 
> Okay, it's a bit more complicated than that. The gpiochip_get_desc()
> shouldn't be used. Bart, what can you suggest to do here? Opencoding
> it doesn't sound to me a (fully) correct approach in a long term.

Note that I can't use gpiochip_request_own_desc(), nor any other
function that calls gpio_request_commit() (like gpiod_get()), because
that checks for gpiochip_line_is_valid(), and this returns false for
the TRNG line, cause that line is not a GPIO line, but interrupt only
line.

That is why I used
  irq = irq_create_mapping(dev, mcu->gc.irq.domain, irq_idx);
until v7, with no reference to gpio descriptors, since this line is not
a GPIO line.

We have discussed this back in April, in the thread
  https://lore.kernel.org/soc/20240418121116.22184-8-kabel@kernel.org/
where we concluded that
  irq = gpiod_to_irq(gpiochip_get_desc(gc, irq_idx));
is better...

Marek

Re: [PATCH v11 6/8] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG

[Marek Behún]

On Wed, 5 Jun 2024 22:00:20 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> > +#include <linux/bitfield.h>
> > +#include <linux/completion.h>  
> 
> + errno.h

I use -EIO, -EINVAL and -ENOMEM in turris-omnia-mcu-base.c,
-EINVAL, -ENOTSUPP in turris-omnia-mcu-gpio.c.

Should I include errno.h in those also? Or is this only needed for
-ERESTARTSYS?

Marek

Re: [PATCH v11 6/8] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG

[Andy Shevchenko]

On Thu, Jun 06, 2024 at 11:11:13AM +0200, Marek Behún wrote:
> On Wed, 5 Jun 2024 22:00:20 +0300
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> 
> > > +#include <linux/bitfield.h>
> > > +#include <linux/completion.h>
> > 
> > + errno.h
> 
> I use -EIO, -EINVAL and -ENOMEM in turris-omnia-mcu-base.c,
> -EINVAL, -ENOTSUPP in turris-omnia-mcu-gpio.c.

> Should I include errno.h in those also?

If you have err.h, then no (it includes asm/errno.h), otherwise, yes.

> Or is this only needed for -ERESTARTSYS?

Definitely yes for Linux internal error codes (>= 512).
Note, that ENOTSUPP is also Linux internal code.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v11 6/8] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG

[Andy Shevchenko]

On Thu, Jun 06, 2024 at 10:53:08AM +0200, Marek Behún wrote:
> On Wed, 5 Jun 2024 22:00:20 +0300
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> 
> > > +       irq_idx = omnia_int_to_gpio_idx[__bf_shf(OMNIA_INT_TRNG)];
> > > +       irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
> > > +       if (irq < 0)
> > > +               return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");  
> > 
> > Okay, it's a bit more complicated than that. The gpiochip_get_desc()
> > shouldn't be used. Bart, what can you suggest to do here? Opencoding
> > it doesn't sound to me a (fully) correct approach in a long term.
> 
> Note that I can't use gpiochip_request_own_desc(), nor any other
> function that calls gpio_request_commit() (like gpiod_get()), because
> that checks for gpiochip_line_is_valid(), and this returns false for
> the TRNG line, cause that line is not a GPIO line, but interrupt only
> line.
> 
> That is why I used
>   irq = irq_create_mapping(dev, mcu->gc.irq.domain, irq_idx);
> until v7, with no reference to gpio descriptors, since this line is not
> a GPIO line.
> 
> We have discussed this back in April, in the thread
>   https://lore.kernel.org/soc/20240418121116.22184-8-kabel@kernel.org/
> where we concluded that
>   irq = gpiod_to_irq(gpiochip_get_desc(gc, irq_idx));
> is better...

That's fine to not use other APIs, the problem here is with reference counting
on the GPIO device. The API you could use is gpio_device_get_desc(). But you
need to have a GPIO device pointer somewhere in your driver being available.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v11 6/8] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG

[Marek Behún]

On Thu, 6 Jun 2024 13:11:09 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Thu, Jun 06, 2024 at 10:53:08AM +0200, Marek Behún wrote:
> > On Wed, 5 Jun 2024 22:00:20 +0300
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> >   
> > > > +       irq_idx = omnia_int_to_gpio_idx[__bf_shf(OMNIA_INT_TRNG)];
> > > > +       irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
> > > > +       if (irq < 0)
> > > > +               return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");    
> > > 
> > > Okay, it's a bit more complicated than that. The gpiochip_get_desc()
> > > shouldn't be used. Bart, what can you suggest to do here? Opencoding
> > > it doesn't sound to me a (fully) correct approach in a long term.  
> > 
> > Note that I can't use gpiochip_request_own_desc(), nor any other
> > function that calls gpio_request_commit() (like gpiod_get()), because
> > that checks for gpiochip_line_is_valid(), and this returns false for
> > the TRNG line, cause that line is not a GPIO line, but interrupt only
> > line.
> > 
> > That is why I used
> >   irq = irq_create_mapping(dev, mcu->gc.irq.domain, irq_idx);
> > until v7, with no reference to gpio descriptors, since this line is not
> > a GPIO line.
> > 
> > We have discussed this back in April, in the thread
> >   https://lore.kernel.org/soc/20240418121116.22184-8-kabel@kernel.org/
> > where we concluded that
> >   irq = gpiod_to_irq(gpiochip_get_desc(gc, irq_idx));
> > is better...  
> 
> That's fine to not use other APIs, the problem here is with reference counting
> on the GPIO device. The API you could use is gpio_device_get_desc(). But you
> need to have a GPIO device pointer somewhere in your driver being available.

Rewriting to gpio_device_get_desc() is simple, since
  gpiochip_get_desc(gc, hwnum)
is equivalent to
  gpio_device_get_desc(gc->gpiodev, hwnum)

Obviously neither of these take care of reference counting. But what
reference counting are you talking about?
Is it the
  try_module_get(desc->gdev->owner)
in gpiod_request()?
Or are we talking only about the FLAG_REQUESTED flag on the descriptor
flags? (This one should not be needed since the GPIO line cannot be
requested, becuase it is not a valid GPIO line.)

Since the line is not a valid GPIO line, I thought that we don't need
to refcount in GPIO code. gpiod_to_irq() will call the gpiochip's
.to_irq() method, which will call gpiochip_to_irq(), which will call
irq_create_mapping()
  gpiod_to_irq()
    gpiochip_to_irq()
      irq_create_mapping()

Then on gpiochip removal, the gpiochip_irqchip_remove() manually
disposes all IRQ mappings with irq_dispose_mapping().

Marek

Re: [PATCH v11 6/8] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG

[Herbert Xu]

On Wed, Jun 05, 2024 at 06:18:49PM +0200, Marek Behún wrote:
>
> +static int omnia_trng_read(struct hwrng *rng, void *data, size_t max, bool wait)
> +{
> +	struct omnia_mcu *mcu = (struct omnia_mcu *)rng->priv;

Please don't cast rng->priv in this manner.  Please take a look at
drivers/char/hw_random/bcm2835-rng.c for how it should be done.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH v11 6/8] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG

[Marek Behún]

On Fri, 7 Jun 2024 18:30:24 +0800
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Wed, Jun 05, 2024 at 06:18:49PM +0200, Marek Behún wrote:
> >
> > +static int omnia_trng_read(struct hwrng *rng, void *data, size_t max, bool wait)
> > +{
> > +	struct omnia_mcu *mcu = (struct omnia_mcu *)rng->priv;  
> 
> Please don't cast rng->priv in this manner.  Please take a look at
> drivers/char/hw_random/bcm2835-rng.c for how it should be done.
> 
> Thanks,

THX, prepared for next version.

Re: [PATCH v11 6/8] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG

[Bartosz Golaszewski]

On Wed, Jun 5, 2024 at 9:00 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Wed, Jun 5, 2024 at 7:19 PM Marek Behún <kabel@kernel.org> wrote:
> >
> > Add support for true random number generator provided by the MCU.
> > New Omnia boards come without the Atmel SHA204-A chip. Instead the
> > crypto functionality is provided by new microcontroller, which has
> > a TRNG peripheral.
>
> +Cc: Bart for gpiochip_get_desc() usage.
>
> ...
>
> > +#include <linux/bitfield.h>
> > +#include <linux/completion.h>
>
> + errno.h
>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/gpio/driver.h>
> > +#include <linux/hw_random.h>
> > +#include <linux/i2c.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/minmax.h>
> > +#include <linux/module.h>
> > +#include <linux/string.h>
>
> > +#include <linux/turris-omnia-mcu-interface.h>
>
> As per other patches.
>
> > +#include <linux/types.h>
> > +
> > +#include "turris-omnia-mcu.h"
>
> ...
>
> > +       irq_idx = omnia_int_to_gpio_idx[__bf_shf(OMNIA_INT_TRNG)];
> > +       irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
> > +       if (irq < 0)
> > +               return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");
>
> Okay, it's a bit more complicated than that. The gpiochip_get_desc()
> shouldn't be used. Bart, what can you suggest to do here? Opencoding
> it doesn't sound to me a (fully) correct approach in a long term.
>

Andy's worried about reference counting of the GPIO device. Maybe you
should just ref the GPIO device in irq_request_resources() and unref
it in irq_release_resources()? Then you could use gpiochip_get_desc()
just fine.

Bart

Re: [PATCH v11 6/8] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG

[Marek Behún]

On Mon, 17 Jun 2024 10:38:31 +0200
Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> On Wed, Jun 5, 2024 at 9:00 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Wed, Jun 5, 2024 at 7:19 PM Marek Behún <kabel@kernel.org> wrote:  
> > >
> > > Add support for true random number generator provided by the MCU.
> > > New Omnia boards come without the Atmel SHA204-A chip. Instead the
> > > crypto functionality is provided by new microcontroller, which has
> > > a TRNG peripheral.  
> >
> > +Cc: Bart for gpiochip_get_desc() usage.
> >
> > ...
> >  
> > > +#include <linux/bitfield.h>
> > > +#include <linux/completion.h>  
> >
> > + errno.h
> >  
> > > +#include <linux/gpio/consumer.h>
> > > +#include <linux/gpio/driver.h>
> > > +#include <linux/hw_random.h>
> > > +#include <linux/i2c.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/minmax.h>
> > > +#include <linux/module.h>
> > > +#include <linux/string.h>  
> >  
> > > +#include <linux/turris-omnia-mcu-interface.h>  
> >
> > As per other patches.
> >  
> > > +#include <linux/types.h>
> > > +
> > > +#include "turris-omnia-mcu.h"  
> >
> > ...
> >  
> > > +       irq_idx = omnia_int_to_gpio_idx[__bf_shf(OMNIA_INT_TRNG)];
> > > +       irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
> > > +       if (irq < 0)
> > > +               return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");  
> >
> > Okay, it's a bit more complicated than that. The gpiochip_get_desc()
> > shouldn't be used. Bart, what can you suggest to do here? Opencoding
> > it doesn't sound to me a (fully) correct approach in a long term.
> >  
> 
> Andy's worried about reference counting of the GPIO device. Maybe you
> should just ref the GPIO device in irq_request_resources() and unref
> it in irq_release_resources()? Then you could use gpiochip_get_desc()
> just fine.

But this is already being done.

The irqchip uses GPIOCHIP_IRQ_RESOURCE_HELPERS macro in its definition:

  static const struct irq_chip omnia_mcu_irq_chip = {
    ...
    GPIOCHIP_IRQ_RESOURCE_HELPERS,
  };

This means that gpiochip_irq_reqres() and gpiochip_irq_relres() are
used as irq_request_resources() and irq_release_resources().

The gpiochip_reqres_irq() code increases the module refcount and even
locks the line as IRQ:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib.c?h=v6.10-rc4#n3732

Marek

Re: [PATCH v11 6/8] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG

[Bartosz Golaszewski]

On Mon, Jun 17, 2024 at 10:56 AM Marek Behún <kabel@kernel.org> wrote:
>
> On Mon, 17 Jun 2024 10:38:31 +0200
> Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > On Wed, Jun 5, 2024 at 9:00 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > >
> > > On Wed, Jun 5, 2024 at 7:19 PM Marek Behún <kabel@kernel.org> wrote:
> > > >
> > > > Add support for true random number generator provided by the MCU.
> > > > New Omnia boards come without the Atmel SHA204-A chip. Instead the
> > > > crypto functionality is provided by new microcontroller, which has
> > > > a TRNG peripheral.
> > >
> > > +Cc: Bart for gpiochip_get_desc() usage.
> > >
> > > ...
> > >
> > > > +#include <linux/bitfield.h>
> > > > +#include <linux/completion.h>
> > >
> > > + errno.h
> > >
> > > > +#include <linux/gpio/consumer.h>
> > > > +#include <linux/gpio/driver.h>
> > > > +#include <linux/hw_random.h>
> > > > +#include <linux/i2c.h>
> > > > +#include <linux/interrupt.h>
> > > > +#include <linux/minmax.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/string.h>
> > >
> > > > +#include <linux/turris-omnia-mcu-interface.h>
> > >
> > > As per other patches.
> > >
> > > > +#include <linux/types.h>
> > > > +
> > > > +#include "turris-omnia-mcu.h"
> > >
> > > ...
> > >
> > > > +       irq_idx = omnia_int_to_gpio_idx[__bf_shf(OMNIA_INT_TRNG)];
> > > > +       irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
> > > > +       if (irq < 0)
> > > > +               return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");
> > >
> > > Okay, it's a bit more complicated than that. The gpiochip_get_desc()
> > > shouldn't be used. Bart, what can you suggest to do here? Opencoding
> > > it doesn't sound to me a (fully) correct approach in a long term.
> > >
> >
> > Andy's worried about reference counting of the GPIO device. Maybe you
> > should just ref the GPIO device in irq_request_resources() and unref
> > it in irq_release_resources()? Then you could use gpiochip_get_desc()
> > just fine.
>
> But this is already being done.
>
> The irqchip uses GPIOCHIP_IRQ_RESOURCE_HELPERS macro in its definition:
>
>   static const struct irq_chip omnia_mcu_irq_chip = {
>     ...
>     GPIOCHIP_IRQ_RESOURCE_HELPERS,
>   };
>
> This means that gpiochip_irq_reqres() and gpiochip_irq_relres() are
> used as irq_request_resources() and irq_release_resources().
>
> The gpiochip_reqres_irq() code increases the module refcount and even
> locks the line as IRQ:
>
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib.c?h=v6.10-rc4#n3732
>
> Marek

Andy: what is the issue here then exactly?

Bart

Re: [PATCH v11 6/8] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG

[Andy Shevchenko]

On Mon, Jun 17, 2024 at 11:07 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> On Mon, Jun 17, 2024 at 10:56 AM Marek Behún <kabel@kernel.org> wrote:
> > On Mon, 17 Jun 2024 10:38:31 +0200
> > Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > On Wed, Jun 5, 2024 at 9:00 PM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Wed, Jun 5, 2024 at 7:19 PM Marek Behún <kabel@kernel.org> wrote:
> > > > >
> > > > > Add support for true random number generator provided by the MCU.
> > > > > New Omnia boards come without the Atmel SHA204-A chip. Instead the
> > > > > crypto functionality is provided by new microcontroller, which has
> > > > > a TRNG peripheral.
> > > >
> > > > +Cc: Bart for gpiochip_get_desc() usage.

...

> > > > > +       irq_idx = omnia_int_to_gpio_idx[__bf_shf(OMNIA_INT_TRNG)];
> > > > > +       irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
> > > > > +       if (irq < 0)
> > > > > +               return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");
> > > >
> > > > Okay, it's a bit more complicated than that. The gpiochip_get_desc()
> > > > shouldn't be used. Bart, what can you suggest to do here? Opencoding
> > > > it doesn't sound to me a (fully) correct approach in a long term.
> > >
> > > Andy's worried about reference counting of the GPIO device. Maybe you
> > > should just ref the GPIO device in irq_request_resources() and unref
> > > it in irq_release_resources()? Then you could use gpiochip_get_desc()
> > > just fine.
> >
> > But this is already being done.
> >
> > The irqchip uses GPIOCHIP_IRQ_RESOURCE_HELPERS macro in its definition:
> >
> >   static const struct irq_chip omnia_mcu_irq_chip = {
> >     ...
> >     GPIOCHIP_IRQ_RESOURCE_HELPERS,
> >   };
> >
> > This means that gpiochip_irq_reqres() and gpiochip_irq_relres() are
> > used as irq_request_resources() and irq_release_resources().
> >
> > The gpiochip_reqres_irq() code increases the module refcount and even
> > locks the line as IRQ:
> >
> >   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib.c?h=v6.10-rc4#n3732
>
> Andy: what is the issue here then exactly?

The function in use is marked as DEPRECATED. If you are fine with its
usage in this case, I have no issues with it.
If you want it to be replaced with the respective
gpio_device_get_desc(), it's fine, but then the question is how
properly get a pointer to GPIO device object.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v11 6/8] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG

[Marek Behún]

On Mon, 17 Jun 2024 12:42:41 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Mon, Jun 17, 2024 at 11:07 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > On Mon, Jun 17, 2024 at 10:56 AM Marek Behún <kabel@kernel.org> wrote:  
> > > On Mon, 17 Jun 2024 10:38:31 +0200
> > > Bartosz Golaszewski <brgl@bgdev.pl> wrote:  
> > > > On Wed, Jun 5, 2024 at 9:00 PM Andy Shevchenko
> > > > <andy.shevchenko@gmail.com> wrote:  
> > > > > On Wed, Jun 5, 2024 at 7:19 PM Marek Behún <kabel@kernel.org> wrote:  
> > > > > >
> > > > > > Add support for true random number generator provided by the MCU.
> > > > > > New Omnia boards come without the Atmel SHA204-A chip. Instead the
> > > > > > crypto functionality is provided by new microcontroller, which has
> > > > > > a TRNG peripheral.  
> > > > >
> > > > > +Cc: Bart for gpiochip_get_desc() usage.  
> 
> ...
> 
> > > > > > +       irq_idx = omnia_int_to_gpio_idx[__bf_shf(OMNIA_INT_TRNG)];
> > > > > > +       irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
> > > > > > +       if (irq < 0)
> > > > > > +               return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");  
> > > > >
> > > > > Okay, it's a bit more complicated than that. The gpiochip_get_desc()
> > > > > shouldn't be used. Bart, what can you suggest to do here? Opencoding
> > > > > it doesn't sound to me a (fully) correct approach in a long term.  
> > > >
> > > > Andy's worried about reference counting of the GPIO device. Maybe you
> > > > should just ref the GPIO device in irq_request_resources() and unref
> > > > it in irq_release_resources()? Then you could use gpiochip_get_desc()
> > > > just fine.  
> > >
> > > But this is already being done.
> > >
> > > The irqchip uses GPIOCHIP_IRQ_RESOURCE_HELPERS macro in its definition:
> > >
> > >   static const struct irq_chip omnia_mcu_irq_chip = {
> > >     ...
> > >     GPIOCHIP_IRQ_RESOURCE_HELPERS,
> > >   };
> > >
> > > This means that gpiochip_irq_reqres() and gpiochip_irq_relres() are
> > > used as irq_request_resources() and irq_release_resources().
> > >
> > > The gpiochip_reqres_irq() code increases the module refcount and even
> > > locks the line as IRQ:
> > >
> > >   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib.c?h=v6.10-rc4#n3732  
> >
> > Andy: what is the issue here then exactly?  
> 
> The function in use is marked as DEPRECATED. If you are fine with its
> usage in this case, I have no issues with it.
> If you want it to be replaced with the respective
> gpio_device_get_desc(), it's fine, but then the question is how
> properly get a pointer to GPIO device object.
> 

Aha, I did not notice that the function is deprecated.

What about

  irq = gpiod_to_irq(gpio_device_get_desc(mcu->gc.gpiodev, irq_idx));

?

Note: I would prefer
  irq_create_mapping(mcu->gc.irq.domain, irq_idx)
since the irq_idx line is not a valid GPIO line and at this part of
the driver the fact that the IRQs are provided through a gpiochip are
semantically irrelevant (we are interested in "an IRQ", not "an IRQ from
a GPIO").

Marek

Re: [PATCH v11 6/8] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG

[Bartosz Golaszewski]

On Mon, Jun 17, 2024 at 1:34 PM Marek Behún <kabel@kernel.org> wrote:
>
> On Mon, 17 Jun 2024 12:42:41 +0200
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>
> > On Mon, Jun 17, 2024 at 11:07 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > On Mon, Jun 17, 2024 at 10:56 AM Marek Behún <kabel@kernel.org> wrote:
> > > > On Mon, 17 Jun 2024 10:38:31 +0200
> > > > Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > > > On Wed, Jun 5, 2024 at 9:00 PM Andy Shevchenko
> > > > > <andy.shevchenko@gmail.com> wrote:
> > > > > > On Wed, Jun 5, 2024 at 7:19 PM Marek Behún <kabel@kernel.org> wrote:
> > > > > > >
> > > > > > > Add support for true random number generator provided by the MCU.
> > > > > > > New Omnia boards come without the Atmel SHA204-A chip. Instead the
> > > > > > > crypto functionality is provided by new microcontroller, which has
> > > > > > > a TRNG peripheral.
> > > > > >
> > > > > > +Cc: Bart for gpiochip_get_desc() usage.
> >
> > ...
> >
> > > > > > > +       irq_idx = omnia_int_to_gpio_idx[__bf_shf(OMNIA_INT_TRNG)];
> > > > > > > +       irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
> > > > > > > +       if (irq < 0)
> > > > > > > +               return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");
> > > > > >
> > > > > > Okay, it's a bit more complicated than that. The gpiochip_get_desc()
> > > > > > shouldn't be used. Bart, what can you suggest to do here? Opencoding
> > > > > > it doesn't sound to me a (fully) correct approach in a long term.
> > > > >
> > > > > Andy's worried about reference counting of the GPIO device. Maybe you
> > > > > should just ref the GPIO device in irq_request_resources() and unref
> > > > > it in irq_release_resources()? Then you could use gpiochip_get_desc()
> > > > > just fine.
> > > >
> > > > But this is already being done.
> > > >
> > > > The irqchip uses GPIOCHIP_IRQ_RESOURCE_HELPERS macro in its definition:
> > > >
> > > >   static const struct irq_chip omnia_mcu_irq_chip = {
> > > >     ...
> > > >     GPIOCHIP_IRQ_RESOURCE_HELPERS,
> > > >   };
> > > >
> > > > This means that gpiochip_irq_reqres() and gpiochip_irq_relres() are
> > > > used as irq_request_resources() and irq_release_resources().
> > > >
> > > > The gpiochip_reqres_irq() code increases the module refcount and even
> > > > locks the line as IRQ:
> > > >
> > > >   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib.c?h=v6.10-rc4#n3732
> > >
> > > Andy: what is the issue here then exactly?
> >
> > The function in use is marked as DEPRECATED. If you are fine with its
> > usage in this case, I have no issues with it.
> > If you want it to be replaced with the respective
> > gpio_device_get_desc(), it's fine, but then the question is how
> > properly get a pointer to GPIO device object.
> >
>
> Aha, I did not notice that the function is deprecated.
>
> What about
>
>   irq = gpiod_to_irq(gpio_device_get_desc(mcu->gc.gpiodev, irq_idx));
>
> ?
>
> Note: I would prefer
>   irq_create_mapping(mcu->gc.irq.domain, irq_idx)
> since the irq_idx line is not a valid GPIO line and at this part of
> the driver the fact that the IRQs are provided through a gpiochip are
> semantically irrelevant (we are interested in "an IRQ", not "an IRQ from
> a GPIO").
>
> Marek

The reason to deprecate it was the fact that it's dangerous to use
from outside of the GPIO provider code. I actually plan to soon make
this function private to gpiolib, there's just one pinctrl driver left
to convert.

So maybe it's better to not use it here. Please keep in mind that
gpio_device_get_desc() doesn't increase the reference count of
gpio_device so you need to make sure it stays alive. But it seems to
be the case here as you're within the driver code still.

Bart