soc.lore.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 00/11] Turris Omnia MCU driver
@ 2024-04-18 12:11 Marek Behún
  2024-04-18 12:11 ` [PATCH v6 01/11] dt-bindings: arm: add cznic,turris-omnia-mcu binding Marek Behún
                   ` (11 more replies)
  0 siblings, 12 replies; 36+ messages in thread
From: Marek Behún @ 2024-04-18 12:11 UTC (permalink / raw)
  To: Gregory CLEMENT, Arnd Bergmann, soc, Alessandro Zummo,
	Alexandre Belloni, Andy Shevchenko, Bartosz Golaszewski,
	Christophe JAILLET, Dan Carpenter, devicetree,
	Greg Kroah-Hartman, Guenter Roeck, Hans de Goede, Herbert Xu,
	Krzysztof Kozlowski, Linus Walleij, linux-crypto, linux-gpio,
	linux-rtc, linux-watchdog, Matti Vaittinen, Olivia Mackall,
	Rob Herring, Wim Van Sebroeck
  Cc: arm, Marek Behún, Andrew Lunn, Conor Dooley,
	Krzysztof Kozlowski, Rob Herring, Sebastian Hesselbarth,
	Uwe Kleine-König

Hello Andy, Dan, Linus, Arnd, Gregory, and others,

I am sending v6 of the series adding Turris Omnia MCU driver.

This series depends on the immutable branch between LEDs and locking,
introducing devm_mutex_init(), see the PR
  https://lore.kernel.org/linux-leds/20240412084616.GR2399047@google.com/

See the cover letters for v1, v2, v3, v4 and v5:
  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/

Changes since v5:
- use dev_get_drvdata(dev) instead of
  i2c_get_clientdata(to_i2c_client(dev))
- uses devm_mutex_init() from the above mentioned branch
- addressed Andy's suggestions for patch 2 (excluding the one suggesting
  100 column lines, I still prefer 80 columns)
- addressed Dan's suggestion for devm_irq_create_mapping() (return
  -ENXIO if mapping could not be created, not 0 as irq_create_mapping()
  does)
- I threw away the changes for other drivers that came with the new
  devm_ helpers, notably conversion to devm_debugfs_create_dir(), as
  there were some problems

Marek Behún (11):
  dt-bindings: arm: 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
  devm-helpers: Add resource managed version of irq_create_mapping()
  platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG
  devm-helpers: Add resource managed version of debugfs directory create
    function
  platform: cznic: turris-omnia-mcu: Add support for digital message
    signing via debugfs
  ARM: dts: turris-omnia: Add MCU system-controller node
  ARM: dts: turris-omnia: Add GPIO key node for front button

 .../ABI/testing/debugfs-turris-omnia-mcu      |   13 +
 .../sysfs-bus-i2c-devices-turris-omnia-mcu    |  126 ++
 .../bindings/arm/cznic,turris-omnia-mcu.yaml  |   86 ++
 MAINTAINERS                                   |    5 +
 .../dts/marvell/armada-385-turris-omnia.dts   |   35 +-
 drivers/platform/Kconfig                      |    2 +
 drivers/platform/Makefile                     |    1 +
 drivers/platform/cznic/Kconfig                |   51 +
 drivers/platform/cznic/Makefile               |    9 +
 .../platform/cznic/turris-omnia-mcu-base.c    |  439 +++++++
 .../platform/cznic/turris-omnia-mcu-debugfs.c |  209 ++++
 .../platform/cznic/turris-omnia-mcu-gpio.c    | 1047 +++++++++++++++++
 .../cznic/turris-omnia-mcu-sys-off-wakeup.c   |  258 ++++
 .../platform/cznic/turris-omnia-mcu-trng.c    |   89 ++
 .../cznic/turris-omnia-mcu-watchdog.c         |  123 ++
 drivers/platform/cznic/turris-omnia-mcu.h     |  188 +++
 include/linux/devm-helpers.h                  |   94 ++
 include/linux/turris-omnia-mcu-interface.h    |  249 ++++
 18 files changed, 3023 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/debugfs-turris-omnia-mcu
 create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu
 create mode 100644 Documentation/devicetree/bindings/arm/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-debugfs.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.43.2


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v6 01/11] dt-bindings: arm: add cznic,turris-omnia-mcu binding
  2024-04-18 12:11 [PATCH v6 00/11] Turris Omnia MCU driver Marek Behún
@ 2024-04-18 12:11 ` Marek Behún
  2024-04-18 15:43   ` Conor Dooley
  2024-04-18 12:11 ` [PATCH v6 02/11] platform: cznic: Add preliminary support for Turris Omnia MCU Marek Behún
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 36+ messages in thread
From: Marek Behún @ 2024-04-18 12:11 UTC (permalink / raw)
  To: Gregory CLEMENT, Arnd Bergmann, soc
  Cc: arm, Marek Behún, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, devicetree, Krzysztof Kozlowski

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>
---
 .../bindings/arm/cznic,turris-omnia-mcu.yaml  | 86 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 87 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/cznic,turris-omnia-mcu.yaml

diff --git a/Documentation/devicetree/bindings/arm/cznic,turris-omnia-mcu.yaml b/Documentation/devicetree/bindings/arm/cznic,turris-omnia-mcu.yaml
new file mode 100644
index 000000000000..dd9ee21ee24d
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/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/arm/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 c23fda1aa1f0..23637ff1e0bc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2141,6 +2141,7 @@ W:	https://www.turris.cz/
 F:	Documentation/ABI/testing/debugfs-moxtet
 F:	Documentation/ABI/testing/sysfs-bus-moxtet-devices
 F:	Documentation/ABI/testing/sysfs-firmware-turris-mox-rwtm
+F:	Documentation/devicetree/bindings/arm/cznic,turris-omnia-mcu.yaml
 F:	Documentation/devicetree/bindings/bus/moxtet.txt
 F:	Documentation/devicetree/bindings/firmware/cznic,turris-mox-rwtm.txt
 F:	Documentation/devicetree/bindings/gpio/gpio-moxtet.txt
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v6 02/11] platform: cznic: Add preliminary support for Turris Omnia MCU
  2024-04-18 12:11 [PATCH v6 00/11] Turris Omnia MCU driver Marek Behún
  2024-04-18 12:11 ` [PATCH v6 01/11] dt-bindings: arm: add cznic,turris-omnia-mcu binding Marek Behún
@ 2024-04-18 12:11 ` Marek Behún
  2024-04-18 12:11 ` [PATCH v6 03/11] platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs Marek Behún
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 36+ messages in thread
From: Marek Behún @ 2024-04-18 12:11 UTC (permalink / raw)
  To: Gregory CLEMENT, Arnd Bergmann, soc, Linus Walleij,
	Bartosz Golaszewski, Andy Shevchenko, linux-gpio,
	Alessandro Zummo, Alexandre Belloni, linux-rtc, Wim Van Sebroeck,
	Guenter Roeck, linux-watchdog
  Cc: arm, 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                |  26 ++
 drivers/platform/cznic/Makefile               |   4 +
 .../platform/cznic/turris-omnia-mcu-base.c    | 376 ++++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |  50 +++
 include/linux/turris-omnia-mcu-interface.h    | 249 ++++++++++++
 9 files changed, 792 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..ff5e7cb00206
--- /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:		July 2024
+KernelVersion:	6.10
+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:		July 2024
+KernelVersion:	6.10
+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:		July 2024
+KernelVersion:	6.10
+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:		July 2024
+KernelVersion:	6.10
+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:		July 2024
+KernelVersion:	6.10
+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:		July 2024
+KernelVersion:	6.10
+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:		July 2024
+KernelVersion:	6.10
+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:		July 2024
+KernelVersion:	6.10
+Contact:	Marek Behún <kabel@kernel.org>
+Description:	(RO) Contains the 64 bit long 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 23637ff1e0bc..b37f7ca81785 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2139,6 +2139,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/arm/cznic,turris-omnia-mcu.yaml
@@ -2152,10 +2153,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 868b20361769..fef907a94001 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 41640172975a..8bf189264374 100644
--- a/drivers/platform/Makefile
+++ b/drivers/platform/Makefile
@@ -10,4 +10,5 @@ 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/
diff --git a/drivers/platform/cznic/Kconfig b/drivers/platform/cznic/Kconfig
new file mode 100644
index 000000000000..f8560ff9c1af
--- /dev/null
+++ b/drivers/platform/cznic/Kconfig
@@ -0,0 +1,26 @@
+# 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"
+	depends on MACH_ARMADA_38X || COMPILE_TEST
+	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..8893c990853a
--- /dev/null
+++ b/drivers/platform/cznic/turris-omnia-mcu-base.c
@@ -0,0 +1,376 @@
+// 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_read(const struct i2c_client *client, u8 cmd, void *reply,
+		   unsigned int len)
+{
+	struct i2c_msg msgs[2];
+	int ret;
+
+	msgs[0].addr = client->addr;
+	msgs[0].flags = 0;
+	msgs[0].len = 1;
+	msgs[0].buf = &cmd;
+	msgs[1].addr = client->addr;
+	msgs[1].flags = I2C_M_RD;
+	msgs[1].len = len;
+	msgs[1].buf = reply;
+
+	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+	if (ret < 0)
+		return ret;
+	if (ret != ARRAY_SIZE(msgs))
+		return -EIO;
+
+	return 0;
+}
+
+static int omnia_get_version_hash(struct omnia_mcu *mcu, bool bootloader,
+				  u8 version[static OMNIA_FW_VERSION_HEX_LEN])
+{
+	u8 reply[OMNIA_FW_VERSION_LEN];
+	int err;
+
+	err = omnia_cmd_read(mcu->client,
+			     bootloader ? CMD_GET_FW_VERSION_BOOT
+					: CMD_GET_FW_VERSION_APP,
+			     reply, sizeof(reply));
+	if (err)
+		return err;
+
+	version[OMNIA_FW_VERSION_HEX_LEN - 1] = '\0';
+	bin2hex(version, reply, OMNIA_FW_VERSION_LEN);
+
+	return 0;
+}
+
+static ssize_t fw_version_hash_show(struct device *dev, char *buf,
+				    bool bootloader)
+{
+	struct omnia_mcu *mcu = dev_get_drvdata(dev);
+	u8 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)
+{
+	int ret;
+
+	ret = omnia_cmd_read_u8(to_i2c_client(dev), CMD_GET_RESET);
+	if (ret < 0)
+		return ret;
+
+	return sysfs_emit(buf, "%d\n", ret);
+}
+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);
+	umode_t mode = a->mode;
+
+	if ((a == &dev_attr_serial_number.attr ||
+	     a == &dev_attr_first_mac_address.attr ||
+	     a == &dev_attr_board_revision.attr) &&
+	    !(mcu->features & FEAT_BOARD_INFO))
+		mode = 0;
+
+	return 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;
+	u8 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(uint16_t status)
+{
+	switch (status & STS_MCU_TYPE_MASK) {
+	case STS_MCU_TYPE_STM32:
+		return "STM32";
+	case STS_MCU_TYPE_GD32:
+		return "GD32";
+	case 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 {
+		uint16_t mask;
+		const char *name;
+	} features[] = {
+		{ FEAT_EXT_CMDS,	   "extended control and status" },
+		{ FEAT_WDT_PING,	   "watchdog pinging" },
+		{ FEAT_LED_STATE_EXT_MASK, "peripheral LED pins reading" },
+		{ FEAT_NEW_INT_API,	   "new interrupt API" },
+		{ FEAT_POWEROFF_WAKEUP,	   "poweroff and wakeup" },
+		{ FEAT_TRNG,		   "true random number generator" },
+	};
+	struct device *dev = &mcu->client->dev;
+	bool suggest_fw_upgrade = false;
+	int status;
+
+	/* status word holds MCU type, which we need below */
+	status = omnia_cmd_read_u16(mcu->client, CMD_GET_STATUS_WORD);
+	if (status < 0)
+		return status;
+
+	/* check whether MCU firmware supports the CMD_GET_FEAUTRES command */
+	if (status & STS_FEATURES_SUPPORTED) {
+		__le32 reply;
+		int ret;
+
+		/* try read 32-bit features */
+		ret = omnia_cmd_read(mcu->client, CMD_GET_FEATURES, &reply,
+				     sizeof(reply));
+		if (ret) {
+			/* try read 16-bit features */
+			ret = omnia_cmd_read_u16(mcu->client, CMD_GET_FEATURES);
+			if (ret < 0)
+				return ret;
+
+			mcu->features = ret;
+		} else {
+			mcu->features = le32_to_cpu(reply);
+
+			if (mcu->features & FEAT_FROM_BIT_16_INVALID)
+				mcu->features &= GENMASK(15, 0);
+		}
+	} else {
+		omnia_info_missing_feature(dev, "feature reading");
+		suggest_fw_upgrade = true;
+	}
+
+	mcu->type = omnia_status_to_mcu_type(status);
+	dev_info(dev, "MCU type %s%s\n", mcu->type,
+		 (mcu->features & FEAT_PERIPH_MCU) ?
+			", with peripheral resets wired" : "");
+
+	omnia_mcu_print_version_hash(mcu, true);
+
+	if (mcu->features & 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, 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]);
+	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 & 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..5b21fbca8fe1
--- /dev/null
+++ b/drivers/platform/cznic/turris-omnia-mcu.h
@@ -0,0 +1,50 @@
+/* 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/i2c.h>
+#include <linux/if_ether.h>
+#include <linux/types.h>
+#include <asm/byteorder.h>
+
+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_read(const struct i2c_client *client, u8 cmd, void *reply,
+		   unsigned int len);
+
+static inline int omnia_cmd_read_u16(const struct i2c_client *client, u8 cmd)
+{
+	__le16 reply;
+	int err;
+
+	err = omnia_cmd_read(client, cmd, &reply, sizeof(reply));
+
+	return err ?: le16_to_cpu(reply);
+}
+
+static inline int omnia_cmd_read_u8(const struct i2c_client *client, u8 cmd)
+{
+	u8 reply;
+	int err;
+
+	err = omnia_cmd_read(client, cmd, &reply, sizeof(reply));
+
+	return err ?: 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..88f8490b5897
--- /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 {
+	CMD_GET_STATUS_WORD		= 0x01, /* slave sends status word back */
+	CMD_GENERAL_CONTROL		= 0x02,
+	CMD_LED_MODE			= 0x03, /* default/user */
+	CMD_LED_STATE			= 0x04, /* LED on/off */
+	CMD_LED_COLOR			= 0x05, /* LED number + RED + GREEN + BLUE */
+	CMD_USER_VOLTAGE		= 0x06,
+	CMD_SET_BRIGHTNESS		= 0x07,
+	CMD_GET_BRIGHTNESS		= 0x08,
+	CMD_GET_RESET			= 0x09,
+	CMD_GET_FW_VERSION_APP		= 0x0A, /* 20B git hash number */
+	CMD_SET_WATCHDOG_STATE		= 0x0B, /* 0 - disable
+						 * 1 - enable / ping
+						 * after boot watchdog is started
+						 * with 2 minutes timeout
+						 */
+
+	/* CMD_WATCHDOG_STATUS		= 0x0C, not implemented anymore */
+
+	CMD_GET_WATCHDOG_STATE		= 0x0D,
+	CMD_GET_FW_VERSION_BOOT		= 0x0E, /* 20B git hash number */
+	CMD_GET_FW_CHECKSUM		= 0x0F, /* 4B length, 4B checksum */
+
+	/* available if FEATURES_SUPPORTED bit set in status word */
+	CMD_GET_FEATURES		= 0x10,
+
+	/* available if EXT_CMD bit set in features */
+	CMD_GET_EXT_STATUS_DWORD	= 0x11,
+	CMD_EXT_CONTROL			= 0x12,
+	CMD_GET_EXT_CONTROL_STATUS	= 0x13,
+
+	/* available if NEW_INT_API bit set in features */
+	CMD_GET_INT_AND_CLEAR		= 0x14,
+	CMD_GET_INT_MASK		= 0x15,
+	CMD_SET_INT_MASK		= 0x16,
+
+	/* available if FLASHING bit set in features */
+	CMD_FLASH			= 0x19,
+
+	/* available if WDT_PING bit set in features */
+	CMD_SET_WDT_TIMEOUT		= 0x20,
+	CMD_GET_WDT_TIMELEFT		= 0x21,
+
+	/* available if POWEROFF_WAKEUP bit set in features */
+	CMD_SET_WAKEUP			= 0x22,
+	CMD_GET_UPTIME_AND_WAKEUP	= 0x23,
+	CMD_POWER_OFF			= 0x24,
+
+	/* available if USB_OVC_PROT_SETTING bit set in features */
+	CMD_SET_USB_OVC_PROT		= 0x25,
+	CMD_GET_USB_OVC_PROT		= 0x26,
+
+	/* available if TRNG bit set in features */
+	CMD_TRNG_COLLECT_ENTROPY	= 0x28,
+
+	/* available if CRYPTO bit set in features */
+	CMD_CRYPTO_GET_PUBLIC_KEY	= 0x29,
+	CMD_CRYPTO_SIGN_MESSAGE		= 0x2A,
+	CMD_CRYPTO_COLLECT_SIGNATURE	= 0x2B,
+
+	/* available if BOARD_INFO it set in features */
+	CMD_BOARD_INFO_GET		= 0x2C,
+	CMD_BOARD_INFO_BURN		= 0x2D,
+
+	/* available only at address 0x2b (led-controller) */
+	/* available only if LED_GAMMA_CORRECTION bit set in features */
+	CMD_SET_GAMMA_CORRECTION	= 0x30,
+	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 */
+	CMD_SET_LED_CORRECTIONS		= 0x32,
+	CMD_GET_LED_CORRECTIONS		= 0x33,
+};
+
+enum omnia_flashing_commands_e {
+	FLASH_CMD_UNLOCK		= 0x01,
+	FLASH_CMD_SIZE_AND_CSUM		= 0x02,
+	FLASH_CMD_PROGRAM		= 0x03,
+	FLASH_CMD_RESET			= 0x04,
+};
+
+enum omnia_sts_word_e {
+	STS_MCU_TYPE_MASK			= GENMASK(1, 0),
+	STS_MCU_TYPE_STM32			= 0 << 0,
+	STS_MCU_TYPE_GD32			= 1 << 0,
+	STS_MCU_TYPE_MKL			= 2 << 0,
+	STS_FEATURES_SUPPORTED			= BIT(2),
+	STS_USER_REGULATOR_NOT_SUPPORTED	= BIT(3),
+	STS_CARD_DET				= BIT(4),
+	STS_MSATA_IND				= BIT(5),
+	STS_USB30_OVC				= BIT(6),
+	STS_USB31_OVC				= BIT(7),
+	STS_USB30_PWRON				= BIT(8),
+	STS_USB31_PWRON				= BIT(9),
+	STS_ENABLE_4V5				= BIT(10),
+	STS_BUTTON_MODE				= BIT(11),
+	STS_BUTTON_PRESSED			= BIT(12),
+	STS_BUTTON_COUNTER_MASK			= GENMASK(15, 13)
+};
+
+enum omnia_ctl_byte_e {
+	CTL_LIGHT_RST		= BIT(0),
+	CTL_HARD_RST		= BIT(1),
+	/* BIT(2) is currently reserved */
+	CTL_USB30_PWRON		= BIT(3),
+	CTL_USB31_PWRON		= BIT(4),
+	CTL_ENABLE_4V5		= BIT(5),
+	CTL_BUTTON_MODE		= BIT(6),
+	CTL_BOOTLOADER		= BIT(7)
+};
+
+enum omnia_features_e {
+	FEAT_PERIPH_MCU			= BIT(0),
+	FEAT_EXT_CMDS			= BIT(1),
+	FEAT_WDT_PING			= BIT(2),
+	FEAT_LED_STATE_EXT_MASK		= GENMASK(4, 3),
+	FEAT_LED_STATE_EXT		= 1 << 3,
+	FEAT_LED_STATE_EXT_V32		= 2 << 3,
+	FEAT_LED_GAMMA_CORRECTION	= BIT(5),
+	FEAT_NEW_INT_API		= BIT(6),
+	FEAT_BOOTLOADER			= BIT(7),
+	FEAT_FLASHING			= BIT(8),
+	FEAT_NEW_MESSAGE_API		= BIT(9),
+	FEAT_BRIGHTNESS_INT		= BIT(10),
+	FEAT_POWEROFF_WAKEUP		= BIT(11),
+	FEAT_CAN_OLD_MESSAGE_API	= BIT(12),
+	FEAT_TRNG			= BIT(13),
+	FEAT_CRYPTO			= BIT(14),
+	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 */
+	FEAT_MCU_TYPE_MASK		= GENMASK(17, 16),
+	FEAT_MCU_TYPE_STM32		= 0 << 16,
+	FEAT_MCU_TYPE_GD32		= 1 << 16,
+	FEAT_MCU_TYPE_MKL		= 2 << 16,
+	FEAT_FEATURES_SUPPORTED		= BIT(18),
+	FEAT_USER_REGULATOR_NOT_SUPPORTED = BIT(19),
+
+	/* must not be set */
+	FEAT_FROM_BIT_16_INVALID	= BIT(20),
+
+	FEAT_PER_LED_CORRECTION		= BIT(21),
+	FEAT_USB_OVC_PROT_SETTING	= BIT(22),
+};
+
+enum omnia_ext_sts_dword_e {
+	EXT_STS_SFP_nDET		= BIT(0),
+	EXT_STS_LED_STATES_MASK		= GENMASK(31, 12),
+	EXT_STS_WLAN0_MSATA_LED		= BIT(12),
+	EXT_STS_WLAN1_LED		= BIT(13),
+	EXT_STS_WLAN2_LED		= BIT(14),
+	EXT_STS_WPAN0_LED		= BIT(15),
+	EXT_STS_WPAN1_LED		= BIT(16),
+	EXT_STS_WPAN2_LED		= BIT(17),
+	EXT_STS_WAN_LED0		= BIT(18),
+	EXT_STS_WAN_LED1		= BIT(19),
+	EXT_STS_LAN0_LED0		= BIT(20),
+	EXT_STS_LAN0_LED1		= BIT(21),
+	EXT_STS_LAN1_LED0		= BIT(22),
+	EXT_STS_LAN1_LED1		= BIT(23),
+	EXT_STS_LAN2_LED0		= BIT(24),
+	EXT_STS_LAN2_LED1		= BIT(25),
+	EXT_STS_LAN3_LED0		= BIT(26),
+	EXT_STS_LAN3_LED1		= BIT(27),
+	EXT_STS_LAN4_LED0		= BIT(28),
+	EXT_STS_LAN4_LED1		= BIT(29),
+	EXT_STS_LAN5_LED0		= BIT(30),
+	EXT_STS_LAN5_LED1		= BIT(31),
+};
+
+enum omnia_ext_ctl_e {
+	EXT_CTL_nRES_MMC		= BIT(0),
+	EXT_CTL_nRES_LAN		= BIT(1),
+	EXT_CTL_nRES_PHY		= BIT(2),
+	EXT_CTL_nPERST0			= BIT(3),
+	EXT_CTL_nPERST1			= BIT(4),
+	EXT_CTL_nPERST2			= BIT(5),
+	EXT_CTL_PHY_SFP			= BIT(6),
+	EXT_CTL_PHY_SFP_AUTO		= BIT(7),
+	EXT_CTL_nVHV_CTRL		= BIT(8),
+};
+
+enum omnia_int_e {
+	INT_CARD_DET		= BIT(0),
+	INT_MSATA_IND		= BIT(1),
+	INT_USB30_OVC		= BIT(2),
+	INT_USB31_OVC		= BIT(3),
+	INT_BUTTON_PRESSED	= BIT(4),
+	INT_SFP_nDET		= BIT(5),
+	INT_BRIGHTNESS_CHANGED	= BIT(6),
+	INT_TRNG		= BIT(7),
+	INT_MESSAGE_SIGNED	= BIT(8),
+
+	INT_LED_STATES_MASK	= GENMASK(31, 12),
+	INT_WLAN0_MSATA_LED	= BIT(12),
+	INT_WLAN1_LED		= BIT(13),
+	INT_WLAN2_LED		= BIT(14),
+	INT_WPAN0_LED		= BIT(15),
+	INT_WPAN1_LED		= BIT(16),
+	INT_WPAN2_LED		= BIT(17),
+	INT_WAN_LED0		= BIT(18),
+	INT_WAN_LED1		= BIT(19),
+	INT_LAN0_LED0		= BIT(20),
+	INT_LAN0_LED1		= BIT(21),
+	INT_LAN1_LED0		= BIT(22),
+	INT_LAN1_LED1		= BIT(23),
+	INT_LAN2_LED0		= BIT(24),
+	INT_LAN2_LED1		= BIT(25),
+	INT_LAN3_LED0		= BIT(26),
+	INT_LAN3_LED1		= BIT(27),
+	INT_LAN4_LED0		= BIT(28),
+	INT_LAN4_LED1		= BIT(29),
+	INT_LAN5_LED0		= BIT(30),
+	INT_LAN5_LED1		= BIT(31),
+};
+
+enum omnia_cmd_poweroff_e {
+	CMD_POWER_OFF_POWERON_BUTTON	= BIT(0),
+	CMD_POWER_OFF_MAGIC		= 0xdead,
+};
+
+enum cmd_usb_ovc_prot_e {
+	CMD_xET_USB_OVC_PROT_PORT_MASK	= GENMASK(3, 0),
+	CMD_xET_USB_OVC_PROT_ENABLE	= BIT(4),
+};
+
+#endif /* __TURRIS_OMNIA_MCU_INTERFACE_H */
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v6 03/11] platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs
  2024-04-18 12:11 [PATCH v6 00/11] Turris Omnia MCU driver Marek Behún
  2024-04-18 12:11 ` [PATCH v6 01/11] dt-bindings: arm: add cznic,turris-omnia-mcu binding Marek Behún
  2024-04-18 12:11 ` [PATCH v6 02/11] platform: cznic: Add preliminary support for Turris Omnia MCU Marek Behún
@ 2024-04-18 12:11 ` Marek Behún
  2024-04-18 12:11 ` [PATCH v6 04/11] platform: cznic: turris-omnia-mcu: Add support for poweroff and wakeup Marek Behún
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 36+ messages in thread
From: Marek Behún @ 2024-04-18 12:11 UTC (permalink / raw)
  To: Gregory CLEMENT, Arnd Bergmann, soc, Linus Walleij,
	Bartosz Golaszewski, Andy Shevchenko, linux-gpio
  Cc: arm, 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    |   33 +-
 .../platform/cznic/turris-omnia-mcu-gpio.c    | 1047 +++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |   69 +-
 6 files changed, 1165 insertions(+), 16 deletions(-)
 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 ff5e7cb00206..b27b60d00c87 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:		July 2024
+KernelVersion:	6.10
+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:		July 2024
 KernelVersion:	6.10
diff --git a/drivers/platform/cznic/Kconfig b/drivers/platform/cznic/Kconfig
index f8560ff9c1af..3a8c3edcd7e6 100644
--- a/drivers/platform/cznic/Kconfig
+++ b/drivers/platform/cznic/Kconfig
@@ -17,9 +17,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 8893c990853a..708b084cb5da 100644
--- a/drivers/platform/cznic/turris-omnia-mcu-base.c
+++ b/drivers/platform/cznic/turris-omnia-mcu-base.c
@@ -22,25 +22,31 @@
 #define OMNIA_FW_VERSION_HEX_LEN	(2 * OMNIA_FW_VERSION_LEN + 1)
 #define OMNIA_BOARD_INFO_LEN		16
 
-int omnia_cmd_read(const struct i2c_client *client, u8 cmd, void *reply,
-		   unsigned int len)
+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;
+	int ret, num;
 
 	msgs[0].addr = client->addr;
 	msgs[0].flags = 0;
-	msgs[0].len = 1;
-	msgs[0].buf = &cmd;
-	msgs[1].addr = client->addr;
-	msgs[1].flags = I2C_M_RD;
-	msgs[1].len = len;
-	msgs[1].buf = reply;
-
-	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+	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 != ARRAY_SIZE(msgs))
+	if (ret != num)
 		return -EIO;
 
 	return 0;
@@ -188,6 +194,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
 };
 
@@ -353,7 +360,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..b3b203f0d2b9
--- /dev/null
+++ b/drivers/platform/cznic/turris-omnia-mcu-gpio.c
@@ -0,0 +1,1047 @@
+// 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/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 CMD_INT_ARG_LEN			8
+#define FRONT_BUTTON_RELEASE_DELAY	50 /* ms */
+
+static const char * const omnia_mcu_gpio_templates[64] = {
+	/* GPIOs with value read from the 16-bit wide status */
+	[4]  = "gpio%u.MiniPCIe0 Card Detect",
+	[5]  = "gpio%u.MiniPCIe0 mSATA Indicator",
+	[6]  = "gpio%u.Front USB3 port over-current",
+	[7]  = "gpio%u.Rear USB3 port over-current",
+	[8]  = "gpio%u.Front USB3 port power",
+	[9]  = "gpio%u.Rear USB3 port power",
+	[12] = "gpio%u.Front Button",
+
+	/* GPIOs with value read from the 32-bit wide extended status */
+	[16] = "gpio%u.SFP nDET",
+	[28] = "gpio%u.MiniPCIe0 LED",
+	[29] = "gpio%u.MiniPCIe1 LED",
+	[30] = "gpio%u.MiniPCIe2 LED",
+	[31] = "gpio%u.MiniPCIe0 PAN LED",
+	[32] = "gpio%u.MiniPCIe1 PAN LED",
+	[33] = "gpio%u.MiniPCIe2 PAN LED",
+	[34] = "gpio%u.WAN PHY LED0",
+	[35] = "gpio%u.WAN PHY LED1",
+	[36] = "gpio%u.LAN switch p0 LED0",
+	[37] = "gpio%u.LAN switch p0 LED1",
+	[38] = "gpio%u.LAN switch p1 LED0",
+	[39] = "gpio%u.LAN switch p1 LED1",
+	[40] = "gpio%u.LAN switch p2 LED0",
+	[41] = "gpio%u.LAN switch p2 LED1",
+	[42] = "gpio%u.LAN switch p3 LED0",
+	[43] = "gpio%u.LAN switch p3 LED1",
+	[44] = "gpio%u.LAN switch p4 LED0",
+	[45] = "gpio%u.LAN switch p4 LED1",
+	[46] = "gpio%u.LAN switch p5 LED0",
+	[47] = "gpio%u.LAN switch p5 LED1",
+
+	/* GPIOs with value read from the 16-bit wide extended control status */
+	[48] = "gpio%u.eMMC nRESET",
+	[49] = "gpio%u.LAN switch nRESET",
+	[50] = "gpio%u.WAN PHY nRESET",
+	[51] = "gpio%u.MiniPCIe0 nPERST",
+	[52] = "gpio%u.MiniPCIe1 nPERST",
+	[53] = "gpio%u.MiniPCIe2 nPERST",
+	[54] = "gpio%u.WAN PHY SFP mux",
+};
+
+#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,					\
+		.feat = _feat,						\
+		.feat_mask = _feat_mask,				\
+	}
+#define _DEF_GPIO_STS(_name) \
+	_DEF_GPIO(CMD_GET_STATUS_WORD, 0, STS_ ## _name, 0, INT_ ## _name, 0, 0)
+#define _DEF_GPIO_CTL(_name) \
+	_DEF_GPIO(CMD_GET_STATUS_WORD, CMD_GENERAL_CONTROL, STS_ ## _name, \
+		  CTL_ ## _name, 0, 0, 0)
+#define _DEF_GPIO_EXT_STS(_name, _feat) \
+	_DEF_GPIO(CMD_GET_EXT_STATUS_DWORD, 0, EXT_STS_ ## _name, 0, \
+		  INT_ ## _name, FEAT_ ## _feat | FEAT_EXT_CMDS, \
+		  FEAT_ ## _feat | FEAT_EXT_CMDS)
+#define _DEF_GPIO_EXT_STS_LED(_name, _ledext) \
+	_DEF_GPIO(CMD_GET_EXT_STATUS_DWORD, 0, EXT_STS_ ## _name, 0, \
+		  INT_ ## _name, FEAT_LED_STATE_ ## _ledext, \
+		  FEAT_LED_STATE_EXT_MASK)
+#define _DEF_GPIO_EXT_STS_LEDALL(_name) \
+	_DEF_GPIO(CMD_GET_EXT_STATUS_DWORD, 0, EXT_STS_ ## _name, 0, \
+		  INT_ ## _name, FEAT_LED_STATE_EXT_MASK, 0)
+#define _DEF_GPIO_EXT_CTL(_name, _feat) \
+	_DEF_GPIO(CMD_GET_EXT_CONTROL_STATUS, CMD_EXT_CONTROL, \
+		  EXT_CTL_ ## _name, EXT_CTL_ ## _name, 0, \
+		  FEAT_ ## _feat | FEAT_EXT_CMDS, \
+		  FEAT_ ## _feat | FEAT_EXT_CMDS)
+#define _DEF_INT(_name) \
+	_DEF_GPIO(0, 0, 0, 0, INT_ ## _name, 0, 0)
+
+static const struct omnia_gpio {
+	u8 cmd, ctl_cmd;
+	u32 bit, ctl_bit;
+	u32 int_bit;
+	u16 feat, feat_mask;
+} 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(INT_CARD_DET)]		= 4,
+	[__bf_shf(INT_MSATA_IND)]		= 5,
+	[__bf_shf(INT_USB30_OVC)]		= 6,
+	[__bf_shf(INT_USB31_OVC)]		= 7,
+	[__bf_shf(INT_BUTTON_PRESSED)]		= 12,
+	[__bf_shf(INT_TRNG)]			= 13,
+	[__bf_shf(INT_MESSAGE_SIGNED)]		= 14,
+	[__bf_shf(INT_SFP_nDET)]		= 16,
+	[__bf_shf(INT_BRIGHTNESS_CHANGED)]	= 11,
+	[__bf_shf(INT_WLAN0_MSATA_LED)]		= 28,
+	[__bf_shf(INT_WLAN1_LED)]		= 29,
+	[__bf_shf(INT_WLAN2_LED)]		= 30,
+	[__bf_shf(INT_WPAN0_LED)]		= 31,
+	[__bf_shf(INT_WPAN1_LED)]		= 32,
+	[__bf_shf(INT_WPAN2_LED)]		= 33,
+	[__bf_shf(INT_WAN_LED0)]		= 34,
+	[__bf_shf(INT_WAN_LED1)]		= 35,
+	[__bf_shf(INT_LAN0_LED0)]		= 36,
+	[__bf_shf(INT_LAN0_LED1)]		= 37,
+	[__bf_shf(INT_LAN1_LED0)]		= 38,
+	[__bf_shf(INT_LAN1_LED1)]		= 39,
+	[__bf_shf(INT_LAN2_LED0)]		= 40,
+	[__bf_shf(INT_LAN2_LED1)]		= 41,
+	[__bf_shf(INT_LAN3_LED0)]		= 42,
+	[__bf_shf(INT_LAN3_LED1)]		= 43,
+	[__bf_shf(INT_LAN4_LED0)]		= 44,
+	[__bf_shf(INT_LAN4_LED1)]		= 45,
+	[__bf_shf(INT_LAN5_LED0)]		= 46,
+	[__bf_shf(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)
+{
+	size_t len;
+	u8 buf[5];
+
+	buf[0] = cmd;
+
+	switch (cmd) {
+	case CMD_GENERAL_CONTROL:
+		buf[1] = val;
+		buf[2] = mask;
+		len = 3;
+		break;
+
+	case 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,
+						 CMD_GET_EXT_CONTROL_STATUS,
+						 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, CMD_EXT_CONTROL, EXT_CTL_PHY_SFP_AUTO,
+				     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 = gpio->ctl_bit;
+	val = value ? mask : 0;
+
+	if (offset == OMNIA_GPIO_PHY_SFP_OFFSET)
+		mask |= 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 == CMD_GET_STATUS_WORD &&
+	    !(mcu->features & FEAT_NEW_INT_API))
+		return !!(mcu->last_status & gpio->bit);
+
+	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->int_bit && (mcu->is_cached & gpio->int_bit))
+		return !!(mcu->cached & gpio->int_bit);
+
+	return omnia_cmd_read_bit(mcu->client, gpio->cmd, gpio->bit);
+}
+
+static int omnia_gpio_get_multiple(struct gpio_chip *gc, unsigned long *mask,
+				   unsigned long *bits)
+{
+	struct omnia_mcu *mcu = gpiochip_get_data(gc);
+	u32 sts_bits, ext_sts_bits, ext_ctl_bits;
+	int err, i;
+
+	sts_bits = 0;
+	ext_sts_bits = 0;
+	ext_ctl_bits = 0;
+
+	/* determine which bits to read from the 3 possible commands */
+	for_each_set_bit(i, mask, ARRAY_SIZE(omnia_gpios)) {
+		if (omnia_gpios[i].cmd == CMD_GET_STATUS_WORD)
+			sts_bits |= omnia_gpios[i].bit;
+		else if (omnia_gpios[i].cmd == CMD_GET_EXT_STATUS_DWORD)
+			ext_sts_bits |= omnia_gpios[i].bit;
+		else if (omnia_gpios[i].cmd == CMD_GET_EXT_CONTROL_STATUS)
+			ext_ctl_bits |= omnia_gpios[i].bit;
+	}
+
+	guard(mutex)(&mcu->lock);
+
+	if (mcu->features & FEAT_NEW_INT_API) {
+		/* read relevant bits from status */
+		err = omnia_cmd_read_bits(mcu->client, CMD_GET_STATUS_WORD,
+					  sts_bits, &sts_bits);
+		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_bits = mcu->last_status;
+	}
+
+	/* read relevant bits from extended status */
+	err = omnia_cmd_read_bits(mcu->client, CMD_GET_EXT_STATUS_DWORD,
+				  ext_sts_bits, &ext_sts_bits);
+	if (err)
+		return err;
+
+	/* read relevant bits from extended control */
+	err = omnia_cmd_read_bits(mcu->client, CMD_GET_EXT_CONTROL_STATUS,
+				  ext_ctl_bits, &ext_ctl_bits);
+	if (err)
+		return err;
+
+	/* assign relevant bits in result */
+	for_each_set_bit(i, mask, ARRAY_SIZE(omnia_gpios)) {
+		if (omnia_gpios[i].cmd == CMD_GET_STATUS_WORD)
+			__assign_bit(i, bits, sts_bits & omnia_gpios[i].bit);
+		else if (omnia_gpios[i].cmd == CMD_GET_EXT_STATUS_DWORD)
+			__assign_bit(i, bits, ext_sts_bits &
+					      omnia_gpios[i].bit);
+		else if (omnia_gpios[i].cmd == CMD_GET_EXT_CONTROL_STATUS)
+			__assign_bit(i, bits, ext_ctl_bits &
+					      omnia_gpios[i].bit);
+	}
+
+	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 = 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)
+{
+	struct omnia_mcu *mcu = gpiochip_get_data(gc);
+	u16 ext_ctl, ext_ctl_mask;
+	u8 ctl, ctl_mask;
+	int i;
+
+	ctl = 0;
+	ctl_mask = 0;
+	ext_ctl = 0;
+	ext_ctl_mask = 0;
+
+	for_each_set_bit(i, mask, ARRAY_SIZE(omnia_gpios)) {
+		if (omnia_gpios[i].ctl_cmd == CMD_GENERAL_CONTROL) {
+			ctl_mask |= omnia_gpios[i].ctl_bit;
+			if (test_bit(i, bits))
+				ctl |= omnia_gpios[i].ctl_bit;
+		} else if (omnia_gpios[i].ctl_cmd == CMD_EXT_CONTROL) {
+			ext_ctl_mask |= omnia_gpios[i].ctl_bit;
+			if (test_bit(i, bits))
+				ext_ctl |= omnia_gpios[i].ctl_bit;
+		}
+	}
+
+	guard(mutex)(&mcu->lock);
+
+	if (ctl_mask)
+		omnia_ctl_cmd_locked(mcu, CMD_GENERAL_CONTROL, ctl, ctl_mask);
+
+	if (ext_ctl_mask)
+		omnia_ctl_cmd_locked(mcu, 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->int_bit)
+			__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);
+	u32 bit = omnia_gpios[hwirq].int_bit;
+
+	mcu->rising &= ~bit;
+	mcu->falling &= ~bit;
+}
+
+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);
+	u32 bit = omnia_gpios[hwirq].int_bit;
+
+	if (!omnia_gpios[hwirq].cmd)
+		mcu->rising &= ~bit;
+	mcu->mask &= ~bit;
+	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);
+	u32 bit = omnia_gpios[hwirq].int_bit;
+
+	gpiochip_enable_irq(gc, hwirq);
+	mcu->mask |= bit;
+	if (!omnia_gpios[hwirq].cmd)
+		mcu->rising |= bit;
+}
+
+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;
+	u32 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;
+	}
+
+	if (type & IRQ_TYPE_EDGE_RISING)
+		mcu->rising |= bit;
+	else
+		mcu->rising &= ~bit;
+
+	if (type & IRQ_TYPE_EDGE_FALLING)
+		mcu->falling |= bit;
+	else
+		mcu->falling &= ~bit;
+
+	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 & 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 interrupt mask and reports pending interrupt bitmap int this
+ * interleaved format. The rationale behind it 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, u32 rising, u32 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, u32 *rising, u32 *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 + CMD_INT_ARG_LEN];
+	u32 rising, falling;
+	int err;
+
+	/* nothing to do if MCU firmware does not support new interrupt API */
+	if (!(mcu->features & FEAT_NEW_INT_API))
+		return;
+
+	cmd[0] = 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->int_bit)
+			__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 + CMD_INT_ARG_LEN] = {};
+
+	cmd[0] = 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
+ * CMD_GET_INT_AND_CLEAR command in order to retrieve all unmasked interrupts.
+ */
+static size_t omnia_irq_compute_pending_length(u32 rising, u32 falling)
+{
+	size_t rlen = 0, flen = 0;
+
+	if (rising)
+		rlen = ((__fls(rising) >> 3) << 1) + 1;
+
+	if (falling)
+		flen = ((__fls(falling) >> 3) << 1) + 2;
+
+	return max(rlen, flen);
+}
+
+static bool omnia_irq_read_pending_new(struct omnia_mcu *mcu,
+				       unsigned long *pending)
+{
+	struct device *dev = &mcu->client->dev;
+	u8 reply[CMD_INT_ARG_LEN] = {};
+	u32 rising, falling;
+	size_t 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, 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, u16 *status)
+{
+	int ret;
+
+	ret = omnia_cmd_read_u16(mcu->client, CMD_GET_STATUS_WORD);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * Old firmware has a bug wherein it never resets the USB port
+	 * overcurrent bits back to zero. Ignore them.
+	 */
+	*status = ret & ~(STS_USB30_OVC | 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(u32 *rising, u32 *falling, u16 rising_sts,
+			      u16 falling_sts, u16 sts_bit, u32 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)
+{
+	struct device *dev = &mcu->client->dev;
+	u16 status, rising_sts, falling_sts;
+	u32 rising, falling;
+	int ret;
+
+	guard(mutex)(&mcu->lock);
+
+	ret = omnia_read_status_word_old_fw(mcu, &status);
+	if (ret < 0) {
+		dev_err(dev, "Cannot read pending IRQs: %d\n", ret);
+		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 STS_BUTTON_PRESSED bit needs special handling, because the old
+	 * firmware clears the STS_BUTTON_PRESSED bit on successful completion
+	 * of the CMD_GET_STATUS_WORD command, resulting in another interrupt:
+	 * - first we get an interrupt, we read the status word where
+	 *   STS_BUTTON_PRESSED is present,
+	 * - MCU clears the STS_BUTTON_PRESSED bit because we read the status
+	 *   word,
+	 * - we get another interrupt because the status word changed again
+	 *   (STS_BUTTON_PRESSED 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 & 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));
+	} else if (mcu->button_pressed_emul) {
+		status |= 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,
+			  STS_CARD_DET, INT_CARD_DET);
+	fill_int_from_sts(&rising, &falling, rising_sts, falling_sts,
+			  STS_MSATA_IND, INT_MSATA_IND);
+	fill_int_from_sts(&rising, &falling, rising_sts, falling_sts,
+			  STS_BUTTON_PRESSED, 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 & 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[2] = { "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 & FEAT_NEW_INT_API) {
+		val = omnia_cmd_read_bit(mcu->client, CMD_GET_STATUS_WORD,
+					 STS_BUTTON_MODE);
+		if (val < 0)
+			return val;
+	} else {
+		val = !!(mcu->last_status & 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, CMD_GENERAL_CONTROL,
+				   i ? CTL_BUTTON_MODE : 0,
+				   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 & 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 < 0)
+			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 5b21fbca8fe1..e8d9bf1c8fdc 100644
--- a/drivers/platform/cznic/turris-omnia-mcu.h
+++ b/drivers/platform/cznic/turris-omnia-mcu.h
@@ -8,9 +8,12 @@
 #ifndef __TURRIS_OMNIA_MCU_H
 #define __TURRIS_OMNIA_MCU_H
 
-#include <linux/i2c.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 omnia_mcu {
@@ -22,10 +25,66 @@ 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;
+	u32 mask, rising, falling, both, cached, is_cached;
+	/* Old MCU firmware handling needs the following */
+	struct delayed_work button_release_emul_work;
+	u16 last_status;
+	bool button_pressed_emul;
 };
 
-int omnia_cmd_read(const struct i2c_client *client, u8 cmd, void *reply,
-		   unsigned int len);
+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,
+				  size_t 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);
+}
+
+/* Returns 0 on success */
+static inline int omnia_cmd_read_bits(const struct i2c_client *client, u8 cmd,
+				      u32 bits, u32 *dst)
+{
+	__le32 reply;
+	int err;
+
+	if (!bits) {
+		*dst = 0;
+		return 0;
+	}
+
+	err = omnia_cmd_read(client, cmd, &reply, (__fls(bits) >> 3) + 1);
+	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,
+				     u32 bit)
+{
+	u32 reply;
+	int err;
+
+	err = omnia_cmd_read_bits(client, cmd, bit, &reply);
+	if (err)
+		return err;
+
+	return !!reply;
+}
 
 static inline int omnia_cmd_read_u16(const struct i2c_client *client, u8 cmd)
 {
@@ -47,4 +106,8 @@ static inline int omnia_cmd_read_u8(const struct i2c_client *client, u8 cmd)
 	return err ?: 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.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v6 04/11] platform: cznic: turris-omnia-mcu: Add support for poweroff and wakeup
  2024-04-18 12:11 [PATCH v6 00/11] Turris Omnia MCU driver Marek Behún
                   ` (2 preceding siblings ...)
  2024-04-18 12:11 ` [PATCH v6 03/11] platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs Marek Behún
@ 2024-04-18 12:11 ` Marek Behún
  2024-04-18 12:11 ` [PATCH v6 05/11] platform: cznic: turris-omnia-mcu: Add support for MCU watchdog Marek Behún
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 36+ messages in thread
From: Marek Behún @ 2024-04-18 12:11 UTC (permalink / raw)
  To: Gregory CLEMENT, Arnd Bergmann, soc, Alessandro Zummo,
	Alexandre Belloni, Andy Shevchenko, Bartosz Golaszewski,
	Linus Walleij, linux-rtc
  Cc: arm, 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>
---
 .../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   | 258 ++++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |  20 ++
 6 files changed, 304 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 b27b60d00c87..564119849388 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:		July 2024
+KernelVersion:	6.10
+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:		July 2024
 KernelVersion:	6.10
diff --git a/drivers/platform/cznic/Kconfig b/drivers/platform/cznic/Kconfig
index 3a8c3edcd7e6..0a752aa654fa 100644
--- a/drivers/platform/cznic/Kconfig
+++ b/drivers/platform/cznic/Kconfig
@@ -19,10 +19,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 708b084cb5da..697ace090bda 100644
--- a/drivers/platform/cznic/turris-omnia-mcu-base.c
+++ b/drivers/platform/cznic/turris-omnia-mcu-base.c
@@ -195,6 +195,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
 };
 
@@ -360,6 +361,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..fbd1b4cd7011
--- /dev/null
+++ b/drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c
@@ -0,0 +1,258 @@
+// 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, 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, 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, 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 = CMD_POWER_OFF_POWERON_BUTTON;
+	else
+		arg = 0;
+
+	cmd[0] = CMD_POWER_OFF;
+	put_unaligned_le16(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(0xffffffff, (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] = CMD_GENERAL_CONTROL;
+
+	if (reboot_mode == REBOOT_HARD)
+		cmd[1] = cmd[2] = CTL_HARD_RST;
+	else
+		cmd[1] = cmd[2] = 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 & 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 & 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 e8d9bf1c8fdc..6179a4f2069e 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 omnia_mcu {
 	struct i2c_client *client;
@@ -34,6 +36,11 @@ struct omnia_mcu {
 	struct delayed_work button_release_emul_work;
 	u16 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,
@@ -46,6 +53,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)
 {
@@ -107,7 +125,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.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v6 05/11] platform: cznic: turris-omnia-mcu: Add support for MCU watchdog
  2024-04-18 12:11 [PATCH v6 00/11] Turris Omnia MCU driver Marek Behún
                   ` (3 preceding siblings ...)
  2024-04-18 12:11 ` [PATCH v6 04/11] platform: cznic: turris-omnia-mcu: Add support for poweroff and wakeup Marek Behún
@ 2024-04-18 12:11 ` Marek Behún
  2024-04-18 13:26   ` Guenter Roeck
  2024-04-18 12:11 ` [PATCH v6 06/11] devm-helpers: Add resource managed version of irq_create_mapping() Marek Behún
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 36+ messages in thread
From: Marek Behún @ 2024-04-18 12:11 UTC (permalink / raw)
  To: Gregory CLEMENT, Arnd Bergmann, soc, Andy Shevchenko,
	Bartosz Golaszewski, Guenter Roeck, Linus Walleij,
	linux-watchdog, Wim Van Sebroeck
  Cc: arm, 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         | 123 ++++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |  24 ++++
 5 files changed, 154 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 0a752aa654fa..e2649cdecc38 100644
--- a/drivers/platform/cznic/Kconfig
+++ b/drivers/platform/cznic/Kconfig
@@ -20,6 +20,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.
@@ -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)
+	  - 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 697ace090bda..5f88119d825c 100644
--- a/drivers/platform/cznic/turris-omnia-mcu-base.c
+++ b/drivers/platform/cznic/turris-omnia-mcu-base.c
@@ -365,6 +365,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..b0de9585da84
--- /dev/null
+++ b/drivers/platform/cznic/turris-omnia-mcu-watchdog.c
@@ -0,0 +1,123 @@
+// 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/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, 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, 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, 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, CMD_SET_WDT_TIMEOUT,
+				   timeout * 10);
+}
+
+static unsigned int omnia_wdt_get_timeleft(struct watchdog_device *wdt)
+{
+	struct omnia_mcu *mcu = watchdog_get_drvdata(wdt);
+	int ret;
+
+	ret = omnia_cmd_read_u16(mcu->client, CMD_GET_WDT_TIMELEFT);
+	if (ret < 0) {
+		dev_err(&mcu->client->dev, "Cannot get watchdog timeleft: %d\n",
+			ret);
+		return 0;
+	}
+
+	return ret / 10;
+}
+
+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;
+	int ret;
+
+	if (!(mcu->features & 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 = 6553; /* 65535 deciseconds */
+
+	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);
+
+	ret = omnia_cmd_read_u8(mcu->client, CMD_GET_WATCHDOG_STATE);
+	if (ret < 0)
+		return dev_err_probe(dev, ret,
+				     "Cannot get MCU watchdog state\n");
+
+	if (ret)
+		set_bit(WDOG_HW_RUNNING, &mcu->wdt.status);
+
+	watchdog_set_nowayout(&mcu->wdt, nowayout);
+	watchdog_stop_on_reboot(&mcu->wdt);
+	ret = devm_watchdog_register_device(dev, &mcu->wdt);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "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 6179a4f2069e..1838cb3d636e 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>
@@ -41,6 +42,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,
@@ -53,6 +57,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)
 {
@@ -129,5 +152,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.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v6 06/11] devm-helpers: Add resource managed version of irq_create_mapping()
  2024-04-18 12:11 [PATCH v6 00/11] Turris Omnia MCU driver Marek Behún
                   ` (4 preceding siblings ...)
  2024-04-18 12:11 ` [PATCH v6 05/11] platform: cznic: turris-omnia-mcu: Add support for MCU watchdog Marek Behún
@ 2024-04-18 12:11 ` Marek Behún
  2024-04-23 12:32   ` Matti Vaittinen
  2024-04-23 12:46   ` Andy Shevchenko
  2024-04-18 12:11 ` [PATCH v6 07/11] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG Marek Behún
                   ` (5 subsequent siblings)
  11 siblings, 2 replies; 36+ messages in thread
From: Marek Behún @ 2024-04-18 12:11 UTC (permalink / raw)
  To: Gregory CLEMENT, Arnd Bergmann, soc, Hans de Goede,
	Matti Vaittinen, Dan Carpenter
  Cc: arm, Marek Behún

Add resource managed version of irq_create_mapping(), to help drivers
automatically dispose a linux irq mapping when driver is detached.

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 include/linux/devm-helpers.h | 54 ++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/include/linux/devm-helpers.h b/include/linux/devm-helpers.h
index 74891802200d..fe62b28baf03 100644
--- a/include/linux/devm-helpers.h
+++ b/include/linux/devm-helpers.h
@@ -24,6 +24,8 @@
  */
 
 #include <linux/device.h>
+#include <linux/kconfig.h>
+#include <linux/irqdomain.h>
 #include <linux/workqueue.h>
 
 static inline void devm_delayed_work_drop(void *res)
@@ -76,4 +78,56 @@ static inline int devm_work_autocancel(struct device *dev,
 	return devm_add_action(dev, devm_work_drop, w);
 }
 
+/**
+ * devm_irq_mapping_drop - devm action for disposing an irq mapping
+ * @res:	linux irq number cast to the void * type
+ *
+ * devm_irq_mapping_drop() can be used as an action parameter for the
+ * devm_add_action_or_reset() function in order to automatically dispose
+ * a linux irq mapping when a device driver is detached.
+ */
+static inline void devm_irq_mapping_drop(void *res)
+{
+	irq_dispose_mapping((unsigned int)(unsigned long)res);
+}
+
+/**
+ * devm_irq_create_mapping - Resource managed version of irq_create_mapping()
+ * @dev:	Device which lifetime the mapping is bound to
+ * @domain:	domain owning this hardware interrupt or NULL for default domain
+ * @hwirq:	hardware irq number in that domain space
+ *
+ * Create an irq mapping to linux irq space which is automatically disposed when
+ * the driver is detached.
+ * devm_irq_create_mapping() can be used to omit the explicit
+ * irq_dispose_mapping() call when driver is detached.
+ *
+ * Returns a linux irq number on success, -ENXIO if the mapping could not be
+ * created, or a negative error number if devm action could not be added.
+ */
+static inline int devm_irq_create_mapping(struct device *dev,
+					  struct irq_domain *domain,
+					  irq_hw_number_t hwirq)
+{
+	unsigned int virq = irq_create_mapping(domain, hwirq);
+
+	if (!virq)
+		return -ENXIO;
+
+	/*
+	 * irq_dispose_mapping() is an empty function if CONFIG_IRQ_DOMAIN is
+	 * disabled. No need to register an action in that case.
+	 */
+	if (IS_ENABLED(CONFIG_IRQ_DOMAIN)) {
+		int err;
+
+		err = devm_add_action_or_reset(dev, devm_irq_mapping_drop,
+					       (void *)(unsigned long)virq);
+		if (err)
+			return err;
+	}
+
+	return virq;
+}
+
 #endif
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v6 07/11] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG
  2024-04-18 12:11 [PATCH v6 00/11] Turris Omnia MCU driver Marek Behún
                   ` (5 preceding siblings ...)
  2024-04-18 12:11 ` [PATCH v6 06/11] devm-helpers: Add resource managed version of irq_create_mapping() Marek Behún
@ 2024-04-18 12:11 ` Marek Behún
  2024-04-23 15:58   ` Andy Shevchenko
  2024-04-18 12:11 ` [PATCH v6 08/11] devm-helpers: Add resource managed version of debugfs directory create function Marek Behún
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 36+ messages in thread
From: Marek Behún @ 2024-04-18 12:11 UTC (permalink / raw)
  To: Gregory CLEMENT, Arnd Bergmann, soc, Andy Shevchenko,
	Olivia Mackall, Herbert Xu, Greg Kroah-Hartman, linux-crypto
  Cc: arm, 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    | 89 +++++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |  8 ++
 6 files changed, 106 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 e2649cdecc38..750d5f47dba8 100644
--- a/drivers/platform/cznic/Kconfig
+++ b/drivers/platform/cznic/Kconfig
@@ -19,6 +19,7 @@ config TURRIS_OMNIA_MCU
 	depends on I2C
 	select GPIOLIB
 	select GPIOLIB_IRQCHIP
+	select HW_RANDOM
 	select RTC_CLASS
 	select WATCHDOG_CORE
 	help
@@ -28,6 +29,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 5f88119d825c..7fe4a3df93a6 100644
--- a/drivers/platform/cznic/turris-omnia-mcu-base.c
+++ b/drivers/platform/cznic/turris-omnia-mcu-base.c
@@ -369,7 +369,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 b3b203f0d2b9..625018ca82cc 100644
--- a/drivers/platform/cznic/turris-omnia-mcu-gpio.c
+++ b/drivers/platform/cznic/turris-omnia-mcu-gpio.c
@@ -162,7 +162,7 @@ static const struct omnia_gpio {
 };
 
 /* 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(INT_CARD_DET)]		= 4,
 	[__bf_shf(INT_MSATA_IND)]		= 5,
 	[__bf_shf(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..24fa8cb5d522
--- /dev/null
+++ b/drivers/platform/cznic/turris-omnia-mcu-trng.c
@@ -0,0 +1,89 @@
+// 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/devm-helpers.h>
+#include <linux/i2c.h>
+#include <linux/irqdomain.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 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 + 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 -EINTR;
+
+		err = omnia_cmd_read(mcu->client, CMD_TRNG_COLLECT_ENTROPY,
+				     reply, sizeof(reply));
+		if (err)
+			return err;
+
+		bytes = min3(reply[0], max, 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;
+	int irq, err;
+	u8 irq_idx;
+
+	if (!(mcu->features & FEAT_TRNG))
+		return 0;
+
+	irq_idx = omnia_int_to_gpio_idx[__bf_shf(INT_TRNG)];
+	irq = devm_irq_create_mapping(dev, mcu->gc.irq.domain, irq_idx);
+	if (irq < 0)
+		return dev_err_probe(dev, irq, "Cannot map TRNG IRQ\n");
+
+	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 1838cb3d636e..e0cf10f8c32e 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>
@@ -45,6 +47,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,
@@ -147,11 +153,13 @@ static inline int omnia_cmd_read_u8(const struct i2c_client *client, u8 cmd)
 	return err ?: 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.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v6 08/11] devm-helpers: Add resource managed version of debugfs directory create function
  2024-04-18 12:11 [PATCH v6 00/11] Turris Omnia MCU driver Marek Behún
                   ` (6 preceding siblings ...)
  2024-04-18 12:11 ` [PATCH v6 07/11] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG Marek Behún
@ 2024-04-18 12:11 ` Marek Behún
  2024-04-23 12:33   ` Matti Vaittinen
  2024-04-23 16:02   ` Andy Shevchenko
  2024-04-18 12:11 ` [PATCH v6 09/11] platform: cznic: turris-omnia-mcu: Add support for digital message signing via debugfs Marek Behún
                   ` (3 subsequent siblings)
  11 siblings, 2 replies; 36+ messages in thread
From: Marek Behún @ 2024-04-18 12:11 UTC (permalink / raw)
  To: Gregory CLEMENT, Arnd Bergmann, soc, Hans de Goede,
	Matti Vaittinen, Dan Carpenter, Christophe JAILLET
  Cc: arm, Marek Behún

A few drivers register a devm action to remove a debugfs directory,
implementing a one-liner function that calls debufs_remove_recursive().
Help drivers avoid this repeated implementations by adding managed
version of debugfs directory create function.

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 include/linux/devm-helpers.h | 40 ++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/include/linux/devm-helpers.h b/include/linux/devm-helpers.h
index fe62b28baf03..58e0a7fbd549 100644
--- a/include/linux/devm-helpers.h
+++ b/include/linux/devm-helpers.h
@@ -23,6 +23,7 @@
  * already ran.
  */
 
+#include <linux/debugfs.h>
 #include <linux/device.h>
 #include <linux/kconfig.h>
 #include <linux/irqdomain.h>
@@ -130,4 +131,43 @@ static inline int devm_irq_create_mapping(struct device *dev,
 	return virq;
 }
 
+static inline void devm_debugfs_dir_recursive_drop(void *res)
+{
+	debugfs_remove_recursive(res);
+}
+
+/**
+ * devm_debugfs_create_dir - Resource managed debugfs directory creation
+ * @dev:	Device which lifetime the directory is bound to
+ * @name:	a pointer to a string containing the name of the directory to
+ *		create
+ * @parent:	a pointer to the parent dentry for this file.  This should be a
+ *		directory dentry if set.  If this parameter is NULL, then the
+ *		directory will be created in the root of the debugfs filesystem.
+ *
+ * Create a debugfs directory which is automatically recursively removed when
+ * the driver is detached. A few drivers create debugfs directories which they
+ * want removed before driver is detached.
+ * devm_debugfs_create_dir() can be used to omit the explicit
+ * debugfs_remove_recursive() call when driver is detached.
+ */
+static inline struct dentry *
+devm_debugfs_create_dir(struct device *dev, const char *name,
+			struct dentry *parent)
+{
+	struct dentry *dentry;
+	int err;
+
+	dentry = debugfs_create_dir(name, parent);
+	if (IS_ERR(dentry))
+		return dentry;
+
+	err = devm_add_action_or_reset(dev, devm_debugfs_dir_recursive_drop,
+				       dentry);
+	if (err < 0)
+		return ERR_PTR(err);
+
+	return dentry;
+}
+
 #endif
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v6 09/11] platform: cznic: turris-omnia-mcu: Add support for digital message signing via debugfs
  2024-04-18 12:11 [PATCH v6 00/11] Turris Omnia MCU driver Marek Behún
                   ` (7 preceding siblings ...)
  2024-04-18 12:11 ` [PATCH v6 08/11] devm-helpers: Add resource managed version of debugfs directory create function Marek Behún
@ 2024-04-18 12:11 ` Marek Behún
  2024-04-18 12:11 ` [PATCH v6 10/11] ARM: dts: turris-omnia: Add MCU system-controller node Marek Behún
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 36+ messages in thread
From: Marek Behún @ 2024-04-18 12:11 UTC (permalink / raw)
  To: Gregory CLEMENT, Arnd Bergmann, soc, Andy Shevchenko,
	Greg Kroah-Hartman, linux-crypto
  Cc: arm, Marek Behún

Add support for digital message signing with private key stored in the
MCU. Boards with MKL MCUs have a NIST256p ECDSA private key created
when manufactured. The private key is not readable from the MCU, but
MCU allows for signing messages with it and retrieving the public key.

As described in a similar commit 50524d787de3 ("firmware:
turris-mox-rwtm: support ECDSA signatures via debugfs"):
  The optimal solution would be to register an akcipher provider via
  kernel's crypto API, but crypto API does not yet support accessing
  akcipher API from userspace (and probably won't for some time, see
  https://www.spinics.net/lists/linux-crypto/msg38388.html).

Therefore we add support for accessing this signature generation
mechanism via debugfs for now, so that userspace can access it.

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 .../ABI/testing/debugfs-turris-omnia-mcu      |  13 ++
 .../sysfs-bus-i2c-devices-turris-omnia-mcu    |  13 ++
 MAINTAINERS                                   |   1 +
 drivers/platform/cznic/Kconfig                |   2 +
 drivers/platform/cznic/Makefile               |   1 +
 .../platform/cznic/turris-omnia-mcu-base.c    |  45 +++-
 .../platform/cznic/turris-omnia-mcu-debugfs.c | 209 ++++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |  23 ++
 8 files changed, 306 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/debugfs-turris-omnia-mcu
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-debugfs.c

diff --git a/Documentation/ABI/testing/debugfs-turris-omnia-mcu b/Documentation/ABI/testing/debugfs-turris-omnia-mcu
new file mode 100644
index 000000000000..1665005c2dcd
--- /dev/null
+++ b/Documentation/ABI/testing/debugfs-turris-omnia-mcu
@@ -0,0 +1,13 @@
+What:		/sys/kernel/debug/turris-omnia-mcu/do_sign
+Date:		July 2024
+KernelVersion:	6.10
+Contact:	Marek Behún <kabel@kernel.org>
+Description:
+
+		======= ===========================================================
+		(Write) Message to sign with the ECDSA private key stored in MCU.
+		        The message must be exactly 32 bytes long (since this is
+		        intended to be a SHA-256 hash).
+		(Read)  The resulting signature, 64 bytes. This contains the R and
+			S values of the ECDSA signature, both in big-endian format.
+		======= ===========================================================
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 564119849388..b239224cf9bc 100644
--- a/Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu
+++ b/Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu
@@ -90,6 +90,19 @@ Description:	(RO) Contains the microcontroller type (STM32, GD32, MKL).
 
 		Format: %s.
 
+What:		/sys/bus/i2c/devices/<mcu_device>/public_key
+Date:		July 2024
+KernelVersion:	6.10
+Contact:	Marek Behún <kabel@kernel.org>
+Description:	(RO) Contains board ECDSA public key.
+
+		Only available if MCU supports signing messages with the ECDSA
+		algorithm. If so, the board has a private key stored in the MCU
+		that was generated during manufacture and cannot be retrieved
+		from the MCU.
+
+		Format: %s.
+
 What:		/sys/bus/i2c/devices/<mcu_device>/reset_selector
 Date:		July 2024
 KernelVersion:	6.10
diff --git a/MAINTAINERS b/MAINTAINERS
index b37f7ca81785..d21084ce32ba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2139,6 +2139,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/debugfs-turris-omnia-mcu
 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
diff --git a/drivers/platform/cznic/Kconfig b/drivers/platform/cznic/Kconfig
index 750d5f47dba8..2d45495c0a2d 100644
--- a/drivers/platform/cznic/Kconfig
+++ b/drivers/platform/cznic/Kconfig
@@ -30,6 +30,8 @@ config TURRIS_OMNIA_MCU
 	    disabled) and the ability to configure wake up from this mode (via
 	    rtcwake)
 	  - true random number generator (if available on the MCU)
+	  - ECDSA message signing with board private key (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 eae4c6b341ff..af9213928404 100644
--- a/drivers/platform/cznic/Makefile
+++ b/drivers/platform/cznic/Makefile
@@ -6,3 +6,4 @@ 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
+turris-omnia-mcu-$(CONFIG_DEBUG_FS) += turris-omnia-mcu-debugfs.o
diff --git a/drivers/platform/cznic/turris-omnia-mcu-base.c b/drivers/platform/cznic/turris-omnia-mcu-base.c
index 7fe4a3df93a6..ec08443551a7 100644
--- a/drivers/platform/cznic/turris-omnia-mcu-base.c
+++ b/drivers/platform/cznic/turris-omnia-mcu-base.c
@@ -159,6 +159,16 @@ static ssize_t board_revision_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(board_revision);
 
+static ssize_t public_key_show(struct device *dev, struct device_attribute *a,
+			       char *buf)
+{
+	struct omnia_mcu *mcu = i2c_get_clientdata(to_i2c_client(dev));
+
+	return sysfs_emit(buf, "%*phN\n", OMNIA_MCU_CRYPTO_PUBLIC_KEY_LEN,
+			  mcu->board_public_key);
+}
+static DEVICE_ATTR_RO(public_key);
+
 static struct attribute *omnia_mcu_base_attrs[] = {
 	&dev_attr_fw_version_hash_application.attr,
 	&dev_attr_fw_version_hash_bootloader.attr,
@@ -168,6 +178,7 @@ static struct attribute *omnia_mcu_base_attrs[] = {
 	&dev_attr_serial_number.attr,
 	&dev_attr_first_mac_address.attr,
 	&dev_attr_board_revision.attr,
+	&dev_attr_public_key.attr,
 	NULL
 };
 
@@ -183,6 +194,9 @@ static umode_t omnia_mcu_base_attrs_visible(struct kobject *kobj,
 	     a == &dev_attr_board_revision.attr) &&
 	    !(mcu->features & FEAT_BOARD_INFO))
 		mode = 0;
+	else if (a == &dev_attr_public_key.attr &&
+		 !(mcu->features & FEAT_CRYPTO))
+		mode = 0;
 
 	return mode;
 }
@@ -333,6 +347,24 @@ static int omnia_mcu_read_board_info(struct omnia_mcu *mcu)
 	return 0;
 }
 
+static int omnia_mcu_read_public_key(struct omnia_mcu *mcu)
+{
+	u8 reply[1 + OMNIA_MCU_CRYPTO_PUBLIC_KEY_LEN];
+	int err;
+
+	err = omnia_cmd_read(mcu->client, CMD_CRYPTO_GET_PUBLIC_KEY, reply,
+			     sizeof(reply));
+	if (err)
+		return err;
+
+	if (reply[0] != OMNIA_MCU_CRYPTO_PUBLIC_KEY_LEN)
+		return -EIO;
+
+	memcpy(mcu->board_public_key, &reply[1], sizeof(mcu->board_public_key));
+
+	return 0;
+}
+
 static int omnia_mcu_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
@@ -361,6 +393,13 @@ static int omnia_mcu_probe(struct i2c_client *client)
 					     "Cannot read board info\n");
 	}
 
+	if (mcu->features & FEAT_CRYPTO) {
+		err = omnia_mcu_read_public_key(mcu);
+		if (err)
+			return dev_err_probe(dev, err,
+					     "Cannot read board public key\n");
+	}
+
 	err = omnia_mcu_register_sys_off_and_wakeup(mcu);
 	if (err)
 		return err;
@@ -373,7 +412,11 @@ static int omnia_mcu_probe(struct i2c_client *client)
 	if (err)
 		return err;
 
-	return omnia_mcu_register_trng(mcu);
+	err = omnia_mcu_register_trng(mcu);
+	if (err)
+		return err;
+
+	return omnia_mcu_register_debugfs(mcu);
 }
 
 static const struct of_device_id of_omnia_mcu_match[] = {
diff --git a/drivers/platform/cznic/turris-omnia-mcu-debugfs.c b/drivers/platform/cznic/turris-omnia-mcu-debugfs.c
new file mode 100644
index 000000000000..484664f8d75c
--- /dev/null
+++ b/drivers/platform/cznic/turris-omnia-mcu-debugfs.c
@@ -0,0 +1,209 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CZ.NIC's Turris Omnia MCU ECDSA message signing via debugfs
+ *
+ * 2024 by Marek Behún <kabel@kernel.org>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/completion.h>
+#include <linux/debugfs.h>
+#include <linux/devm-helpers.h>
+#include <linux/i2c.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/string.h>
+#include <linux/turris-omnia-mcu-interface.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>
+
+#include "turris-omnia-mcu.h"
+
+#define CMD_CRYPTO_SIGN_MESSAGE_LEN	32
+
+enum {
+	SIGN_STATE_CLOSED	= 0,
+	SIGN_STATE_OPEN		= 1,
+	SIGN_STATE_REQUESTED	= 2,
+	SIGN_STATE_COLLECTED	= 3,
+};
+
+static irqreturn_t omnia_msg_signed_irq_handler(int irq, void *dev_id)
+{
+	u8 reply[1 + OMNIA_MCU_CRYPTO_SIGNATURE_LEN];
+	struct omnia_mcu *mcu = dev_id;
+	int err;
+
+	err = omnia_cmd_read(mcu->client, CMD_CRYPTO_COLLECT_SIGNATURE, reply,
+			     sizeof(reply));
+	if (!err && reply[0] != OMNIA_MCU_CRYPTO_SIGNATURE_LEN)
+		err = -EIO;
+
+	guard(mutex)(&mcu->sign_lock);
+
+	if (mcu->sign_state == SIGN_STATE_REQUESTED) {
+		mcu->sign_err = err;
+		if (!err)
+			memcpy(mcu->signature, &reply[1],
+			       OMNIA_MCU_CRYPTO_SIGNATURE_LEN);
+		mcu->sign_state = SIGN_STATE_COLLECTED;
+		complete(&mcu->msg_signed_completion);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int do_sign_open(struct inode *inode, struct file *file)
+{
+	struct omnia_mcu *mcu = inode->i_private;
+
+	guard(mutex)(&mcu->sign_lock);
+
+	/* do_sign is allowed to be opened only once */
+	if (mcu->sign_state != SIGN_STATE_CLOSED)
+		return -EBUSY;
+
+	mcu->sign_state = SIGN_STATE_OPEN;
+
+	file->private_data = mcu;
+
+	return nonseekable_open(inode, file);
+}
+
+static int do_sign_release(struct inode *inode, struct file *file)
+{
+	struct omnia_mcu *mcu = file->private_data;
+
+	guard(mutex)(&mcu->sign_lock);
+
+	mcu->sign_state = SIGN_STATE_CLOSED;
+
+	/* forget signature on release even if it was not read, for security */
+	memzero_explicit(mcu->signature, sizeof(mcu->signature));
+
+	return 0;
+}
+
+static ssize_t do_sign_read(struct file *file, char __user *buf, size_t len,
+			    loff_t *ppos)
+{
+	struct omnia_mcu *mcu = file->private_data;
+
+	/* only allow read of one whole signature */
+	if (len != sizeof(mcu->signature))
+		return -EINVAL;
+
+	scoped_guard(mutex, &mcu->sign_lock)
+		if (mcu->sign_state != SIGN_STATE_REQUESTED &&
+		    mcu->sign_state != SIGN_STATE_COLLECTED)
+			return -ENODATA;
+
+	if (wait_for_completion_interruptible(&mcu->msg_signed_completion))
+		return -EINTR;
+
+	guard(mutex)(&mcu->sign_lock);
+
+	mcu->sign_state = SIGN_STATE_OPEN;
+
+	if (mcu->sign_err)
+		return mcu->sign_err;
+
+	if (copy_to_user(buf, mcu->signature, len))
+		return -EFAULT;
+
+	/* on read forget the signature, for security */
+	memzero_explicit(mcu->signature, sizeof(mcu->signature));
+
+	return len;
+}
+
+static ssize_t do_sign_write(struct file *file, const char __user *buf,
+			     size_t len, loff_t *ppos)
+{
+	u8 cmd[1 + CMD_CRYPTO_SIGN_MESSAGE_LEN], reply;
+	struct omnia_mcu *mcu = file->private_data;
+	int err;
+
+	/*
+	 * the input is a SHA-256 hash of a message to sign, so exactly
+	 * 32 bytes have to be read
+	 */
+	if (len != CMD_CRYPTO_SIGN_MESSAGE_LEN)
+		return -EINVAL;
+
+	cmd[0] = CMD_CRYPTO_SIGN_MESSAGE;
+
+	if (copy_from_user(&cmd[1], buf, len))
+		return -EFAULT;
+
+	guard(mutex)(&mcu->sign_lock);
+
+	if (mcu->sign_state != SIGN_STATE_OPEN)
+		return -EBUSY;
+
+	err = omnia_cmd_write_read(mcu->client, cmd, sizeof(cmd), &reply, 1);
+	if (err)
+		return err;
+
+	if (reply)
+		mcu->sign_state = SIGN_STATE_REQUESTED;
+
+	return reply ? len : -EBUSY;
+}
+
+static const struct file_operations do_sign_fops = {
+	.owner		= THIS_MODULE,
+	.open		= do_sign_open,
+	.read		= do_sign_read,
+	.write		= do_sign_write,
+	.release	= do_sign_release,
+	.llseek		= no_llseek,
+};
+
+int omnia_mcu_register_debugfs(struct omnia_mcu *mcu)
+{
+	struct device *dev = &mcu->client->dev;
+	struct dentry *root, *entry;
+	int irq, err;
+	u8 irq_idx;
+
+	if (!(mcu->features & FEAT_CRYPTO))
+		return 0;
+
+	irq_idx = omnia_int_to_gpio_idx[__bf_shf(INT_MESSAGE_SIGNED)];
+	irq = devm_irq_create_mapping(dev, mcu->gc.irq.domain, irq_idx);
+	if (irq < 0)
+		return dev_err_probe(dev, irq,
+				     "Cannot map MESSAGE_SIGNED IRQ\n");
+
+	err = devm_mutex_init(dev, &mcu->sign_lock);
+	if (err)
+		return err;
+
+	mcu->sign_state = 0;
+
+	init_completion(&mcu->msg_signed_completion);
+
+	err = devm_request_threaded_irq(dev, irq, NULL,
+					omnia_msg_signed_irq_handler,
+					IRQF_ONESHOT,
+					"turris-omnia-mcu-debugfs", mcu);
+	if (err)
+		return dev_err_probe(dev, err,
+				     "Cannot request MESSAGE_SIGNED IRQ\n");
+
+	root = devm_debugfs_create_dir(dev, "turris-omnia-mcu", NULL);
+	if (IS_ERR(root))
+		return dev_err_probe(dev, PTR_ERR(root),
+				     "Cannot create debugfs dir\n");
+
+
+	entry = debugfs_create_file_unsafe("do_sign", 0600, root, mcu,
+					   &do_sign_fops);
+	if (IS_ERR(entry))
+		return dev_err_probe(dev, PTR_ERR(entry),
+				     "Cannot create debugfs entry\n");
+
+	return 0;
+}
diff --git a/drivers/platform/cznic/turris-omnia-mcu.h b/drivers/platform/cznic/turris-omnia-mcu.h
index e0cf10f8c32e..5ce639d5ec67 100644
--- a/drivers/platform/cznic/turris-omnia-mcu.h
+++ b/drivers/platform/cznic/turris-omnia-mcu.h
@@ -21,6 +21,9 @@
 #include <asm/byteorder.h>
 #include <asm/unaligned.h>
 
+#define OMNIA_MCU_CRYPTO_PUBLIC_KEY_LEN	33
+#define OMNIA_MCU_CRYPTO_SIGNATURE_LEN	64
+
 struct omnia_mcu {
 	struct i2c_client *client;
 	const char *type;
@@ -30,6 +33,7 @@ struct omnia_mcu {
 	u64 board_serial_number;
 	u8 board_first_mac[ETH_ALEN];
 	u8 board_revision;
+	u8 board_public_key[OMNIA_MCU_CRYPTO_PUBLIC_KEY_LEN];
 
 	/* GPIO chip */
 	struct gpio_chip gc;
@@ -51,6 +55,16 @@ struct omnia_mcu {
 	/* true random number generator */
 	struct hwrng trng;
 	struct completion trng_completion;
+
+#ifdef CONFIG_DEBUG_FS
+	/* MCU ECDSA message signing via debugfs */
+	struct dentry *debugfs_root;
+	struct completion msg_signed_completion;
+	struct mutex sign_lock;
+	unsigned int sign_state;
+	u8 signature[OMNIA_MCU_CRYPTO_SIGNATURE_LEN];
+	int sign_err;
+#endif
 };
 
 int omnia_cmd_write_read(const struct i2c_client *client,
@@ -162,4 +176,13 @@ 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);
 
+#ifdef CONFIG_DEBUG_FS
+int omnia_mcu_register_debugfs(struct omnia_mcu *mcu);
+#else
+static inline int omnia_mcu_register_debugfs(struct omnia_mcu *mcu)
+{
+	return 0;
+}
+#endif
+
 #endif /* __TURRIS_OMNIA_MCU_H */
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v6 10/11] ARM: dts: turris-omnia: Add MCU system-controller node
  2024-04-18 12:11 [PATCH v6 00/11] Turris Omnia MCU driver Marek Behún
                   ` (8 preceding siblings ...)
  2024-04-18 12:11 ` [PATCH v6 09/11] platform: cznic: turris-omnia-mcu: Add support for digital message signing via debugfs Marek Behún
@ 2024-04-18 12:11 ` Marek Behún
  2024-04-18 12:11 ` [PATCH v6 11/11] ARM: dts: turris-omnia: Add GPIO key node for front button Marek Behún
  2024-04-23 16:05 ` [PATCH v6 00/11] Turris Omnia MCU driver Andy Shevchenko
  11 siblings, 0 replies; 36+ messages in thread
From: Marek Behún @ 2024-04-18 12:11 UTC (permalink / raw)
  To: Gregory CLEMENT, Arnd Bergmann, soc
  Cc: arm, Marek Behún, Andrew Lunn, Sebastian Hesselbarth,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Uwe Kleine-König, devicetree

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

Fixes: 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")
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.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v6 11/11] ARM: dts: turris-omnia: Add GPIO key node for front button
  2024-04-18 12:11 [PATCH v6 00/11] Turris Omnia MCU driver Marek Behún
                   ` (9 preceding siblings ...)
  2024-04-18 12:11 ` [PATCH v6 10/11] ARM: dts: turris-omnia: Add MCU system-controller node Marek Behún
@ 2024-04-18 12:11 ` Marek Behún
  2024-04-23 16:05 ` [PATCH v6 00/11] Turris Omnia MCU driver Andy Shevchenko
  11 siblings, 0 replies; 36+ messages in thread
From: Marek Behún @ 2024-04-18 12:11 UTC (permalink / raw)
  To: Gregory CLEMENT, Arnd Bergmann, soc
  Cc: arm, Marek Behún, Andrew Lunn, Sebastian Hesselbarth,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Uwe Kleine-König, devicetree

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

Fixes: 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")
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.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 05/11] platform: cznic: turris-omnia-mcu: Add support for MCU watchdog
  2024-04-18 12:11 ` [PATCH v6 05/11] platform: cznic: turris-omnia-mcu: Add support for MCU watchdog Marek Behún
@ 2024-04-18 13:26   ` Guenter Roeck
  2024-04-18 14:15     ` Marek Behún
  0 siblings, 1 reply; 36+ messages in thread
From: Guenter Roeck @ 2024-04-18 13:26 UTC (permalink / raw)
  To: Marek Behún
  Cc: Gregory CLEMENT, Arnd Bergmann, soc, Andy Shevchenko,
	Bartosz Golaszewski, Linus Walleij, linux-watchdog,
	Wim Van Sebroeck, arm

On Thu, Apr 18, 2024 at 02:11:10PM +0200, Marek Behún wrote:
> Add support for the watchdog mechanism provided by the MCU.
> 
> Signed-off-by: Marek Behún <kabel@kernel.org>

This driver should reside in driver/watchdog and use a generic API
such as regmap to access chip registers.

Guenter

> ---
>  drivers/platform/cznic/Kconfig                |   2 +
>  drivers/platform/cznic/Makefile               |   1 +
>  .../platform/cznic/turris-omnia-mcu-base.c    |   4 +
>  .../cznic/turris-omnia-mcu-watchdog.c         | 123 ++++++++++++++++++
>  drivers/platform/cznic/turris-omnia-mcu.h     |  24 ++++
>  5 files changed, 154 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 0a752aa654fa..e2649cdecc38 100644
> --- a/drivers/platform/cznic/Kconfig
> +++ b/drivers/platform/cznic/Kconfig
> @@ -20,6 +20,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.
> @@ -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)
> +	  - 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 697ace090bda..5f88119d825c 100644
> --- a/drivers/platform/cznic/turris-omnia-mcu-base.c
> +++ b/drivers/platform/cznic/turris-omnia-mcu-base.c
> @@ -365,6 +365,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..b0de9585da84
> --- /dev/null
> +++ b/drivers/platform/cznic/turris-omnia-mcu-watchdog.c
> @@ -0,0 +1,123 @@
> +// 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/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, 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, 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, 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, CMD_SET_WDT_TIMEOUT,
> +				   timeout * 10);
> +}
> +
> +static unsigned int omnia_wdt_get_timeleft(struct watchdog_device *wdt)
> +{
> +	struct omnia_mcu *mcu = watchdog_get_drvdata(wdt);
> +	int ret;
> +
> +	ret = omnia_cmd_read_u16(mcu->client, CMD_GET_WDT_TIMELEFT);
> +	if (ret < 0) {
> +		dev_err(&mcu->client->dev, "Cannot get watchdog timeleft: %d\n",
> +			ret);
> +		return 0;
> +	}
> +
> +	return ret / 10;
> +}
> +
> +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;
> +	int ret;
> +
> +	if (!(mcu->features & 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 = 6553; /* 65535 deciseconds */
> +
> +	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);
> +
> +	ret = omnia_cmd_read_u8(mcu->client, CMD_GET_WATCHDOG_STATE);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret,
> +				     "Cannot get MCU watchdog state\n");
> +
> +	if (ret)
> +		set_bit(WDOG_HW_RUNNING, &mcu->wdt.status);
> +
> +	watchdog_set_nowayout(&mcu->wdt, nowayout);
> +	watchdog_stop_on_reboot(&mcu->wdt);
> +	ret = devm_watchdog_register_device(dev, &mcu->wdt);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "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 6179a4f2069e..1838cb3d636e 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>
> @@ -41,6 +42,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,
> @@ -53,6 +57,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)
>  {
> @@ -129,5 +152,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.43.2
> 
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 05/11] platform: cznic: turris-omnia-mcu: Add support for MCU watchdog
  2024-04-18 13:26   ` Guenter Roeck
@ 2024-04-18 14:15     ` Marek Behún
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Behún @ 2024-04-18 14:15 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Gregory CLEMENT, Arnd Bergmann, soc, Andy Shevchenko,
	Bartosz Golaszewski, Linus Walleij, linux-watchdog,
	Wim Van Sebroeck, arm

On Thu, 18 Apr 2024 06:26:04 -0700
Guenter Roeck <linux@roeck-us.net> wrote:

> On Thu, Apr 18, 2024 at 02:11:10PM +0200, Marek Behún wrote:
> > Add support for the watchdog mechanism provided by the MCU.
> > 
> > Signed-off-by: Marek Behún <kabel@kernel.org>  
> 
> This driver should reside in driver/watchdog and use a generic API
> such as regmap to access chip registers.

Hi Guenter,

I have these points against that:

- the regmap API is not applicable in a sane way to the Turris Omnia
  MCU interface. I was trying to do this 5 years ago. An explanation
  can be found at
    https://www.spinics.net/lists/linux-leds/msg11583.html
  (the LED driver uses same interface as the MCU driver, LED controller
   is implemented by the MCU).

  so were the watchdog driver a separate module, we would either need
  to rape the regmap API to do this, or come up with a new API specific
  for Turris Omnia, which seems like an unnecessary complication just
  for one device

- this watchdog is specific for Turris Omnia, the driver can not be
  reused anywhere else

- there are several other multifunction drivers in kernel registering
  watchdog that do not live in drivers/watchdog/

- leaving it in platform/cznic/turris-omnia-mcu* keeps all these
  turris-omnia-mcu features together and they are all implemented
  by one .ko module

Marek

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 01/11] dt-bindings: arm: add cznic,turris-omnia-mcu binding
  2024-04-18 12:11 ` [PATCH v6 01/11] dt-bindings: arm: add cznic,turris-omnia-mcu binding Marek Behún
@ 2024-04-18 15:43   ` Conor Dooley
  2024-04-19  8:23     ` Marek Behún
  0 siblings, 1 reply; 36+ messages in thread
From: Conor Dooley @ 2024-04-18 15:43 UTC (permalink / raw)
  To: Marek Behún
  Cc: Gregory CLEMENT, Arnd Bergmann, soc, arm, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, devicetree,
	Krzysztof Kozlowski

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

On Thu, Apr 18, 2024 at 02:11:06PM +0200, Marek Behún wrote:
> 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>
> ---
>  .../bindings/arm/cznic,turris-omnia-mcu.yaml  | 86 +++++++++++++++++++

Why's this in bindings/arm btw? Seems like it is some remote firmware if
it is running off-SoC on an MCU, so either remoteproc or firmware would
make more sense? Tying it to arm at least needs an explanation IMO.

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 01/11] dt-bindings: arm: add cznic,turris-omnia-mcu binding
  2024-04-18 15:43   ` Conor Dooley
@ 2024-04-19  8:23     ` Marek Behún
  2024-04-19 11:14       ` Marek Behún
  0 siblings, 1 reply; 36+ messages in thread
From: Marek Behún @ 2024-04-19  8:23 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Gregory CLEMENT, Arnd Bergmann, soc, arm, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, devicetree,
	Krzysztof Kozlowski

On Thu, Apr 18, 2024 at 04:43:54PM +0100, Conor Dooley wrote:
> On Thu, Apr 18, 2024 at 02:11:06PM +0200, Marek Behún wrote:
> > 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>
> > ---
> >  .../bindings/arm/cznic,turris-omnia-mcu.yaml  | 86 +++++++++++++++++++
> 
> Why's this in bindings/arm btw? Seems like it is some remote firmware if
> it is running off-SoC on an MCU, so either remoteproc or firmware would
> make more sense? Tying it to arm at least needs an explanation IMO.

This was discussed with Krzysztof in v1, you can look it up at
  https://lore.kernel.org/soc/20230824131736.067c40e2@dellmb/

Basically the SoC is an ARM board, and the MCU is also always ARM.

I'm guessing firmware would also make sense...

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 01/11] dt-bindings: arm: add cznic,turris-omnia-mcu binding
  2024-04-19  8:23     ` Marek Behún
@ 2024-04-19 11:14       ` Marek Behún
  2024-04-19 13:52         ` Conor Dooley
  0 siblings, 1 reply; 36+ messages in thread
From: Marek Behún @ 2024-04-19 11:14 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Gregory CLEMENT, Arnd Bergmann, soc, arm, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, devicetree,
	Krzysztof Kozlowski

On Fri, Apr 19, 2024 at 10:23:37AM +0200, Marek Behún wrote:
> On Thu, Apr 18, 2024 at 04:43:54PM +0100, Conor Dooley wrote:
> > On Thu, Apr 18, 2024 at 02:11:06PM +0200, Marek Behún wrote:
> > > 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>
> > > ---
> > >  .../bindings/arm/cznic,turris-omnia-mcu.yaml  | 86 +++++++++++++++++++
> > 
> > Why's this in bindings/arm btw? Seems like it is some remote firmware if
> > it is running off-SoC on an MCU, so either remoteproc or firmware would
> > make more sense? Tying it to arm at least needs an explanation IMO.
> 
> This was discussed with Krzysztof in v1, you can look it up at
>   https://lore.kernel.org/soc/20230824131736.067c40e2@dellmb/
> 
> Basically the SoC is an ARM board, and the MCU is also always ARM.

What I wrote does not make sense. I wanted to say is that the driver
drives the peripherals implemented by the Turris Omnia MCU firmware, and
the Turris Omnia router is based on an ARM SoC, and that the MCU is also
an ARM-based MCU.

> 
> I'm guessing firmware would also make sense...
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 01/11] dt-bindings: arm: add cznic,turris-omnia-mcu binding
  2024-04-19 11:14       ` Marek Behún
@ 2024-04-19 13:52         ` Conor Dooley
  0 siblings, 0 replies; 36+ messages in thread
From: Conor Dooley @ 2024-04-19 13:52 UTC (permalink / raw)
  To: Marek Behún
  Cc: Gregory CLEMENT, Arnd Bergmann, soc, arm, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, devicetree,
	Krzysztof Kozlowski

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

On Fri, Apr 19, 2024 at 01:14:45PM +0200, Marek Behún wrote:
> On Fri, Apr 19, 2024 at 10:23:37AM +0200, Marek Behún wrote:
> > On Thu, Apr 18, 2024 at 04:43:54PM +0100, Conor Dooley wrote:
> > > On Thu, Apr 18, 2024 at 02:11:06PM +0200, Marek Behún wrote:
> > > > 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>
> > > > ---
> > > >  .../bindings/arm/cznic,turris-omnia-mcu.yaml  | 86 +++++++++++++++++++
> > > 
> > > Why's this in bindings/arm btw? Seems like it is some remote firmware if
> > > it is running off-SoC on an MCU, so either remoteproc or firmware would
> > > make more sense? Tying it to arm at least needs an explanation IMO.
> > 
> > This was discussed with Krzysztof in v1, you can look it up at
> >   https://lore.kernel.org/soc/20230824131736.067c40e2@dellmb/
> > 
> > Basically the SoC is an ARM board, and the MCU is also always ARM.
> 
> What I wrote does not make sense. I wanted to say is that the driver
> drives the peripherals implemented by the Turris Omnia MCU firmware, and
> the Turris Omnia router is based on an ARM SoC, and that the MCU is also
> an ARM-based MCU.

Yeah, it didn't really make sense, but I read between the lines.
FWIW, I still don't really think that bindings/arm is the right place
for it.

> 
> > 
> > I'm guessing firmware would also make sense...
> > 

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 06/11] devm-helpers: Add resource managed version of irq_create_mapping()
  2024-04-18 12:11 ` [PATCH v6 06/11] devm-helpers: Add resource managed version of irq_create_mapping() Marek Behún
@ 2024-04-23 12:32   ` Matti Vaittinen
  2024-04-23 12:46   ` Andy Shevchenko
  1 sibling, 0 replies; 36+ messages in thread
From: Matti Vaittinen @ 2024-04-23 12:32 UTC (permalink / raw)
  To: Marek Behún, Gregory CLEMENT, Arnd Bergmann, soc,
	Hans de Goede, Dan Carpenter
  Cc: arm

On 4/18/24 15:11, Marek Behún wrote:
> Add resource managed version of irq_create_mapping(), to help drivers
> automatically dispose a linux irq mapping when driver is detached.
> 
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
>   include/linux/devm-helpers.h | 54 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 54 insertions(+)
> 
> diff --git a/include/linux/devm-helpers.h b/include/linux/devm-helpers.h
> index 74891802200d..fe62b28baf03 100644
> --- a/include/linux/devm-helpers.h
> +++ b/include/linux/devm-helpers.h
> @@ -24,6 +24,8 @@
>    */
>   
>   #include <linux/device.h>
> +#include <linux/kconfig.h>
> +#include <linux/irqdomain.h>

I still think stuffing all unrelated APIs in one header will lead to 
problems because of the sheer amount of unrelated stuff which gets 
included when only some APIs from this file are used.

But as I said during V5 review, I don't want to block this if no one 
else thinks it is a real problem.

Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>

>   #include <linux/workqueue.h>
>   
>   static inline void devm_delayed_work_drop(void *res)
> @@ -76,4 +78,56 @@ static inline int devm_work_autocancel(struct device *dev,
>   	return devm_add_action(dev, devm_work_drop, w);
>   }
>   
> +/**
> + * devm_irq_mapping_drop - devm action for disposing an irq mapping
> + * @res:	linux irq number cast to the void * type
> + *
> + * devm_irq_mapping_drop() can be used as an action parameter for the
> + * devm_add_action_or_reset() function in order to automatically dispose
> + * a linux irq mapping when a device driver is detached.
> + */
> +static inline void devm_irq_mapping_drop(void *res)
> +{
> +	irq_dispose_mapping((unsigned int)(unsigned long)res);
> +}
> +
> +/**
> + * devm_irq_create_mapping - Resource managed version of irq_create_mapping()
> + * @dev:	Device which lifetime the mapping is bound to
> + * @domain:	domain owning this hardware interrupt or NULL for default domain
> + * @hwirq:	hardware irq number in that domain space
> + *
> + * Create an irq mapping to linux irq space which is automatically disposed when
> + * the driver is detached.
> + * devm_irq_create_mapping() can be used to omit the explicit
> + * irq_dispose_mapping() call when driver is detached.
> + *
> + * Returns a linux irq number on success, -ENXIO if the mapping could not be
> + * created, or a negative error number if devm action could not be added.
> + */
> +static inline int devm_irq_create_mapping(struct device *dev,
> +					  struct irq_domain *domain,
> +					  irq_hw_number_t hwirq)
> +{
> +	unsigned int virq = irq_create_mapping(domain, hwirq);
> +
> +	if (!virq)
> +		return -ENXIO;
> +
> +	/*
> +	 * irq_dispose_mapping() is an empty function if CONFIG_IRQ_DOMAIN is
> +	 * disabled. No need to register an action in that case.
> +	 */
> +	if (IS_ENABLED(CONFIG_IRQ_DOMAIN)) {
> +		int err;
> +
> +		err = devm_add_action_or_reset(dev, devm_irq_mapping_drop,
> +					       (void *)(unsigned long)virq);
> +		if (err)
> +			return err;
> +	}
> +
> +	return virq;
> +}
> +
>   #endif

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 08/11] devm-helpers: Add resource managed version of debugfs directory create function
  2024-04-18 12:11 ` [PATCH v6 08/11] devm-helpers: Add resource managed version of debugfs directory create function Marek Behún
@ 2024-04-23 12:33   ` Matti Vaittinen
  2024-04-23 16:02   ` Andy Shevchenko
  1 sibling, 0 replies; 36+ messages in thread
From: Matti Vaittinen @ 2024-04-23 12:33 UTC (permalink / raw)
  To: Marek Behún, Gregory CLEMENT, Arnd Bergmann, soc,
	Hans de Goede, Dan Carpenter, Christophe JAILLET
  Cc: arm

On 4/18/24 15:11, Marek Behún wrote:
> A few drivers register a devm action to remove a debugfs directory,
> implementing a one-liner function that calls debufs_remove_recursive().
> Help drivers avoid this repeated implementations by adding managed
> version of debugfs directory create function.
> 
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
>   include/linux/devm-helpers.h | 40 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 40 insertions(+)
> 
> diff --git a/include/linux/devm-helpers.h b/include/linux/devm-helpers.h
> index fe62b28baf03..58e0a7fbd549 100644
> --- a/include/linux/devm-helpers.h
> +++ b/include/linux/devm-helpers.h
> @@ -23,6 +23,7 @@
>    * already ran.
>    */
>   
> +#include <linux/debugfs.h>

With the same remark that I did for 06/11

Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>

Yours,
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 06/11] devm-helpers: Add resource managed version of irq_create_mapping()
  2024-04-18 12:11 ` [PATCH v6 06/11] devm-helpers: Add resource managed version of irq_create_mapping() Marek Behún
  2024-04-23 12:32   ` Matti Vaittinen
@ 2024-04-23 12:46   ` Andy Shevchenko
  2024-04-23 14:21     ` Marek Behún
  2024-04-23 15:14     ` Marek Behún
  1 sibling, 2 replies; 36+ messages in thread
From: Andy Shevchenko @ 2024-04-23 12:46 UTC (permalink / raw)
  To: Marek Behún
  Cc: Gregory CLEMENT, Arnd Bergmann, soc, Hans de Goede,
	Matti Vaittinen, Dan Carpenter, arm

On Thu, Apr 18, 2024 at 02:11:11PM +0200, Marek Behún wrote:
> Add resource managed version of irq_create_mapping(), to help drivers
> automatically dispose a linux irq mapping when driver is detached.

AFAICS this is a dead code.
Please, drop it from the next version.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 06/11] devm-helpers: Add resource managed version of irq_create_mapping()
  2024-04-23 12:46   ` Andy Shevchenko
@ 2024-04-23 14:21     ` Marek Behún
  2024-04-23 15:14     ` Marek Behún
  1 sibling, 0 replies; 36+ messages in thread
From: Marek Behún @ 2024-04-23 14:21 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Gregory CLEMENT, Arnd Bergmann, soc, Hans de Goede,
	Matti Vaittinen, Dan Carpenter, arm

On Tue, Apr 23, 2024 at 03:46:15PM +0300, Andy Shevchenko wrote:
> On Thu, Apr 18, 2024 at 02:11:11PM +0200, Marek Behún wrote:
> > Add resource managed version of irq_create_mapping(), to help drivers
> > automatically dispose a linux irq mapping when driver is detached.
> 
> AFAICS this is a dead code.
> Please, drop it from the next version.

What do you mean by dead code?


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 06/11] devm-helpers: Add resource managed version of irq_create_mapping()
  2024-04-23 12:46   ` Andy Shevchenko
  2024-04-23 14:21     ` Marek Behún
@ 2024-04-23 15:14     ` Marek Behún
  2024-04-23 15:56       ` Andy Shevchenko
  1 sibling, 1 reply; 36+ messages in thread
From: Marek Behún @ 2024-04-23 15:14 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Gregory CLEMENT, Arnd Bergmann, soc, Hans de Goede,
	Matti Vaittinen, Dan Carpenter, arm

On Tue, 23 Apr 2024 15:46:15 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Thu, Apr 18, 2024 at 02:11:11PM +0200, Marek Behún wrote:
> > Add resource managed version of irq_create_mapping(), to help drivers
> > automatically dispose a linux irq mapping when driver is detached.  
> 
> AFAICS this is a dead code.
> Please, drop it from the next version.

Do you mean that it is not used when this patch is applied? Because it
is used in the very next patch. Should I merge these two patches into
one?

Marek

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 06/11] devm-helpers: Add resource managed version of irq_create_mapping()
  2024-04-23 15:14     ` Marek Behún
@ 2024-04-23 15:56       ` Andy Shevchenko
  0 siblings, 0 replies; 36+ messages in thread
From: Andy Shevchenko @ 2024-04-23 15:56 UTC (permalink / raw)
  To: Marek Behún
  Cc: Gregory CLEMENT, Arnd Bergmann, soc, Hans de Goede,
	Matti Vaittinen, Dan Carpenter, arm

On Tue, Apr 23, 2024 at 05:14:58PM +0200, Marek Behún wrote:
> On Tue, 23 Apr 2024 15:46:15 +0300
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > On Thu, Apr 18, 2024 at 02:11:11PM +0200, Marek Behún wrote:
> > > Add resource managed version of irq_create_mapping(), to help drivers
> > > automatically dispose a linux irq mapping when driver is detached.  
> > 
> > AFAICS this is a dead code.
> > Please, drop it from the next version.

> Do you mean that it is not used when this patch is applied? 

No.

> Because it is used in the very next patch.

I missed that, sorry.

> Should I merge these two patches into one?

No.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 07/11] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG
  2024-04-18 12:11 ` [PATCH v6 07/11] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG Marek Behún
@ 2024-04-23 15:58   ` Andy Shevchenko
  2024-04-23 16:32     ` Marek Behún
  0 siblings, 1 reply; 36+ messages in thread
From: Andy Shevchenko @ 2024-04-23 15:58 UTC (permalink / raw)
  To: Marek Behún
  Cc: Gregory CLEMENT, Arnd Bergmann, soc, Olivia Mackall, Herbert Xu,
	Greg Kroah-Hartman, linux-crypto, arm

On Thu, Apr 18, 2024 at 02:11:12PM +0200, Marek Behún 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.

...

> +int omnia_mcu_register_trng(struct omnia_mcu *mcu)
> +{
> +	struct device *dev = &mcu->client->dev;
> +	int irq, err;
> +	u8 irq_idx;
> +
> +	if (!(mcu->features & FEAT_TRNG))
> +		return 0;

> +	irq_idx = omnia_int_to_gpio_idx[__bf_shf(INT_TRNG)];
> +	irq = devm_irq_create_mapping(dev, mcu->gc.irq.domain, irq_idx);
> +	if (irq < 0)
> +		return dev_err_probe(dev, irq, "Cannot map TRNG IRQ\n");

This looks like some workaround against existing gpiod_to_irq(). Why do you
need this?

> +	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;
> +}

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 08/11] devm-helpers: Add resource managed version of debugfs directory create function
  2024-04-18 12:11 ` [PATCH v6 08/11] devm-helpers: Add resource managed version of debugfs directory create function Marek Behún
  2024-04-23 12:33   ` Matti Vaittinen
@ 2024-04-23 16:02   ` Andy Shevchenko
  2024-04-23 16:20     ` Marek Behún
  2024-04-23 16:21     ` Dan Carpenter
  1 sibling, 2 replies; 36+ messages in thread
From: Andy Shevchenko @ 2024-04-23 16:02 UTC (permalink / raw)
  To: Marek Behún
  Cc: Gregory CLEMENT, Arnd Bergmann, soc, Hans de Goede,
	Matti Vaittinen, Dan Carpenter, Christophe JAILLET, arm

On Thu, Apr 18, 2024 at 02:11:13PM +0200, Marek Behún wrote:
> A few drivers register a devm action to remove a debugfs directory,
> implementing a one-liner function that calls debufs_remove_recursive().
> Help drivers avoid this repeated implementations by adding managed
> version of debugfs directory create function.

...

> +static inline struct dentry *
> +devm_debugfs_create_dir(struct device *dev, const char *name,
> +			struct dentry *parent)
> +{
> +	struct dentry *dentry;
> +	int err;

> +	dentry = debugfs_create_dir(name, parent);
> +	if (IS_ERR(dentry))
> +		return dentry;

Why do we care?

> +	err = devm_add_action_or_reset(dev, devm_debugfs_dir_recursive_drop,
> +				       dentry);
> +	if (err < 0)
> +		return ERR_PTR(err);
> +
> +	return dentry;
> +}

static inline struct dentry *
devm_debugfs_create_dir(struct device *dev, const char *name, struct dentry *parent)
{
	struct dentry *dentry;
	int err;

	dentry = debugfs_create_dir(name, parent);

	err = devm_add_action_or_reset(dev, devm_debugfs_dir_recursive_drop, dentry);
	if (err)
		return ERR_PTR(err);

	return dentry;
}

?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 00/11] Turris Omnia MCU driver
  2024-04-18 12:11 [PATCH v6 00/11] Turris Omnia MCU driver Marek Behún
                   ` (10 preceding siblings ...)
  2024-04-18 12:11 ` [PATCH v6 11/11] ARM: dts: turris-omnia: Add GPIO key node for front button Marek Behún
@ 2024-04-23 16:05 ` Andy Shevchenko
  11 siblings, 0 replies; 36+ messages in thread
From: Andy Shevchenko @ 2024-04-23 16:05 UTC (permalink / raw)
  To: Marek Behún
  Cc: Gregory CLEMENT, Arnd Bergmann, soc, Alessandro Zummo,
	Alexandre Belloni, Bartosz Golaszewski, Christophe JAILLET,
	Dan Carpenter, devicetree, Greg Kroah-Hartman, Guenter Roeck,
	Hans de Goede, Herbert Xu, Krzysztof Kozlowski, Linus Walleij,
	linux-crypto, linux-gpio, linux-rtc, linux-watchdog,
	Matti Vaittinen, Olivia Mackall, Rob Herring, Wim Van Sebroeck,
	arm, Andrew Lunn, Conor Dooley, Krzysztof Kozlowski, Rob Herring,
	Sebastian Hesselbarth, Uwe Kleine-König

On Thu, Apr 18, 2024 at 02:11:05PM +0200, Marek Behún wrote:
> Hello Andy, Dan, Linus, Arnd, Gregory, and others,
> 
> I am sending v6 of the series adding Turris Omnia MCU driver.
> 
> This series depends on the immutable branch between LEDs and locking,
> introducing devm_mutex_init(), see the PR
>   https://lore.kernel.org/linux-leds/20240412084616.GR2399047@google.com/

...

>   devm-helpers: Add resource managed version of irq_create_mapping()
>   devm-helpers: Add resource managed version of debugfs directory create
>     function

IIUC you created them as static inline, the header will become yet another
cumbersome and messy "kernel.h". Can we prevent that?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 08/11] devm-helpers: Add resource managed version of debugfs directory create function
  2024-04-23 16:02   ` Andy Shevchenko
@ 2024-04-23 16:20     ` Marek Behún
  2024-04-23 16:33       ` Andy Shevchenko
  2024-04-23 16:21     ` Dan Carpenter
  1 sibling, 1 reply; 36+ messages in thread
From: Marek Behún @ 2024-04-23 16:20 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Gregory CLEMENT, Arnd Bergmann, soc, Hans de Goede,
	Matti Vaittinen, Dan Carpenter, Christophe JAILLET, arm

On Tue, 23 Apr 2024 19:02:21 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Thu, Apr 18, 2024 at 02:11:13PM +0200, Marek Behún wrote:
> > A few drivers register a devm action to remove a debugfs directory,
> > implementing a one-liner function that calls debufs_remove_recursive().
> > Help drivers avoid this repeated implementations by adding managed
> > version of debugfs directory create function.  
> 
> ...
> 
> > +static inline struct dentry *
> > +devm_debugfs_create_dir(struct device *dev, const char *name,
> > +			struct dentry *parent)
> > +{
> > +	struct dentry *dentry;
> > +	int err;  
> 
> > +	dentry = debugfs_create_dir(name, parent);
> > +	if (IS_ERR(dentry))
> > +		return dentry;  
> 
> Why do we care?
> 
> > +	err = devm_add_action_or_reset(dev, devm_debugfs_dir_recursive_drop,
> > +				       dentry);
> > +	if (err < 0)
> > +		return ERR_PTR(err);
> > +
> > +	return dentry;
> > +}  
> 
> static inline struct dentry *
> devm_debugfs_create_dir(struct device *dev, const char *name, struct dentry *parent)
> {
> 	struct dentry *dentry;
> 	int err;
> 
> 	dentry = debugfs_create_dir(name, parent);
> 
> 	err = devm_add_action_or_reset(dev, devm_debugfs_dir_recursive_drop, dentry);
> 	if (err)
> 		return ERR_PTR(err);

But if the dir creation fails, there is no need to allocate devm
destructor...

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 08/11] devm-helpers: Add resource managed version of debugfs directory create function
  2024-04-23 16:02   ` Andy Shevchenko
  2024-04-23 16:20     ` Marek Behún
@ 2024-04-23 16:21     ` Dan Carpenter
  1 sibling, 0 replies; 36+ messages in thread
From: Dan Carpenter @ 2024-04-23 16:21 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Marek Behún, Gregory CLEMENT, Arnd Bergmann, soc,
	Hans de Goede, Matti Vaittinen, Christophe JAILLET, arm

On Tue, Apr 23, 2024 at 07:02:21PM +0300, Andy Shevchenko wrote:
> On Thu, Apr 18, 2024 at 02:11:13PM +0200, Marek Behún wrote:
> > A few drivers register a devm action to remove a debugfs directory,
> > implementing a one-liner function that calls debufs_remove_recursive().
> > Help drivers avoid this repeated implementations by adding managed
> > version of debugfs directory create function.
> 
> ...
> 
> > +static inline struct dentry *
> > +devm_debugfs_create_dir(struct device *dev, const char *name,
> > +			struct dentry *parent)
> > +{
> > +	struct dentry *dentry;
> > +	int err;
> 
> > +	dentry = debugfs_create_dir(name, parent);
> > +	if (IS_ERR(dentry))
> > +		return dentry;
> 
> Why do we care?

We don't.

I am missing some patches, but I suspect this breaks when
CONFIG_DEBUGFS is disabled.  Errors from debugfs functions are supposed
to be ignored.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 07/11] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG
  2024-04-23 15:58   ` Andy Shevchenko
@ 2024-04-23 16:32     ` Marek Behún
  2024-04-23 16:43       ` Andy Shevchenko
  0 siblings, 1 reply; 36+ messages in thread
From: Marek Behún @ 2024-04-23 16:32 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Gregory CLEMENT, Arnd Bergmann, soc, Olivia Mackall, Herbert Xu,
	Greg Kroah-Hartman, linux-crypto, arm

On Tue, 23 Apr 2024 18:58:19 +0300
Andy Shevchenko <andy@kernel.org> wrote:

> On Thu, Apr 18, 2024 at 02:11:12PM +0200, Marek Behún 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.  
> 
> ...
> 
> > +int omnia_mcu_register_trng(struct omnia_mcu *mcu)
> > +{
> > +	struct device *dev = &mcu->client->dev;
> > +	int irq, err;
> > +	u8 irq_idx;
> > +
> > +	if (!(mcu->features & FEAT_TRNG))
> > +		return 0;  
> 
> > +	irq_idx = omnia_int_to_gpio_idx[__bf_shf(INT_TRNG)];
> > +	irq = devm_irq_create_mapping(dev, mcu->gc.irq.domain, irq_idx);
> > +	if (irq < 0)
> > +		return dev_err_probe(dev, irq, "Cannot map TRNG IRQ\n");  
> 
> This looks like some workaround against existing gpiod_to_irq(). Why do you
> need this?

Hmmm, I thought that would not work because that line is only valid
as an IRQ, not as a GPIO (this is enforced via the valid_mask member of
gpio_chip and gpio_irq_chip).

But looking at the code of gpiolib, if I do
  irq = gpiod_to_irq(gpiochip_get_desc(gc, irq_idx));
the valid_mask is not enforced anywhere.

Is this semantically right to do even in spite of the fact that the
line is not a valid GPIO line?

Marek

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 08/11] devm-helpers: Add resource managed version of debugfs directory create function
  2024-04-23 16:20     ` Marek Behún
@ 2024-04-23 16:33       ` Andy Shevchenko
  2024-04-23 16:43         ` Marek Behún
  0 siblings, 1 reply; 36+ messages in thread
From: Andy Shevchenko @ 2024-04-23 16:33 UTC (permalink / raw)
  To: Marek Behún
  Cc: Gregory CLEMENT, Arnd Bergmann, soc, Hans de Goede,
	Matti Vaittinen, Dan Carpenter, Christophe JAILLET, arm

On Tue, Apr 23, 2024 at 06:20:14PM +0200, Marek Behún wrote:
> On Tue, 23 Apr 2024 19:02:21 +0300
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > On Thu, Apr 18, 2024 at 02:11:13PM +0200, Marek Behún wrote:

...

> > > +static inline struct dentry *
> > > +devm_debugfs_create_dir(struct device *dev, const char *name,
> > > +			struct dentry *parent)
> > > +{
> > > +	struct dentry *dentry;
> > > +	int err;  
> > 
> > > +	dentry = debugfs_create_dir(name, parent);
> > > +	if (IS_ERR(dentry))
> > > +		return dentry;  
> > 
> > Why do we care?
> > 
> > > +	err = devm_add_action_or_reset(dev, devm_debugfs_dir_recursive_drop,
> > > +				       dentry);
> > > +	if (err < 0)
> > > +		return ERR_PTR(err);
> > > +
> > > +	return dentry;
> > > +}  
> > 
> > static inline struct dentry *
> > devm_debugfs_create_dir(struct device *dev, const char *name, struct dentry *parent)
> > {
> > 	struct dentry *dentry;
> > 	int err;
> > 
> > 	dentry = debugfs_create_dir(name, parent);
> > 
> > 	err = devm_add_action_or_reset(dev, devm_debugfs_dir_recursive_drop, dentry);
> > 	if (err)
> > 		return ERR_PTR(err);
> 
> But if the dir creation fails, there is no need to allocate devm
> destructor...

Yes, but why do we care about that?
debugfs is designed the way that we should ignore errors from it.

This discussion let me think, that probably the best solution is to add a field
to struct device_driver to handle this.

debugfs will appear only when device is completely instantiated, removed before
anything, and a lot of other benefits (see the story behind dev_groups as a
reference).

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 07/11] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG
  2024-04-23 16:32     ` Marek Behún
@ 2024-04-23 16:43       ` Andy Shevchenko
  2024-04-23 16:57         ` Marek Behún
  0 siblings, 1 reply; 36+ messages in thread
From: Andy Shevchenko @ 2024-04-23 16:43 UTC (permalink / raw)
  To: Marek Behún
  Cc: Andy Shevchenko, Gregory CLEMENT, Arnd Bergmann, soc,
	Olivia Mackall, Herbert Xu, Greg Kroah-Hartman, linux-crypto,
	arm

On Tue, Apr 23, 2024 at 7:32 PM Marek Behún <kabel@kernel.org> wrote:
> On Tue, 23 Apr 2024 18:58:19 +0300
> Andy Shevchenko <andy@kernel.org> wrote:
> > On Thu, Apr 18, 2024 at 02:11:12PM +0200, Marek Behún wrote:

...

> > > +   irq_idx = omnia_int_to_gpio_idx[__bf_shf(INT_TRNG)];
> > > +   irq = devm_irq_create_mapping(dev, mcu->gc.irq.domain, irq_idx);
> > > +   if (irq < 0)
> > > +           return dev_err_probe(dev, irq, "Cannot map TRNG IRQ\n");
> >
> > This looks like some workaround against existing gpiod_to_irq(). Why do you
> > need this?
>
> Hmmm, I thought that would not work because that line is only valid
> as an IRQ, not as a GPIO (this is enforced via the valid_mask member of
> gpio_chip and gpio_irq_chip).
>
> But looking at the code of gpiolib, if I do
>   irq = gpiod_to_irq(gpiochip_get_desc(gc, irq_idx));
> the valid_mask is not enforced anywhere.

Which one? GPIO has two: one per GPIO realm and one for IRQ domain.

> Is this semantically right to do even in spite of the fact that the
> line is not a valid GPIO line?

Yes. It's orthogonal to that.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 08/11] devm-helpers: Add resource managed version of debugfs directory create function
  2024-04-23 16:33       ` Andy Shevchenko
@ 2024-04-23 16:43         ` Marek Behún
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Behún @ 2024-04-23 16:43 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Gregory CLEMENT, Arnd Bergmann, soc, Hans de Goede,
	Matti Vaittinen, Dan Carpenter, Christophe JAILLET, arm

On Tue, 23 Apr 2024 19:33:20 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Tue, Apr 23, 2024 at 06:20:14PM +0200, Marek Behún wrote:
> > On Tue, 23 Apr 2024 19:02:21 +0300
> > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:  
> > > On Thu, Apr 18, 2024 at 02:11:13PM +0200, Marek Behún wrote:  
> 
> ...
> 
> > > > +static inline struct dentry *
> > > > +devm_debugfs_create_dir(struct device *dev, const char *name,
> > > > +			struct dentry *parent)
> > > > +{
> > > > +	struct dentry *dentry;
> > > > +	int err;    
> > >   
> > > > +	dentry = debugfs_create_dir(name, parent);
> > > > +	if (IS_ERR(dentry))
> > > > +		return dentry;    
> > > 
> > > Why do we care?
> > >   
> > > > +	err = devm_add_action_or_reset(dev, devm_debugfs_dir_recursive_drop,
> > > > +				       dentry);
> > > > +	if (err < 0)
> > > > +		return ERR_PTR(err);
> > > > +
> > > > +	return dentry;
> > > > +}    
> > > 
> > > static inline struct dentry *
> > > devm_debugfs_create_dir(struct device *dev, const char *name, struct dentry *parent)
> > > {
> > > 	struct dentry *dentry;
> > > 	int err;
> > > 
> > > 	dentry = debugfs_create_dir(name, parent);
> > > 
> > > 	err = devm_add_action_or_reset(dev, devm_debugfs_dir_recursive_drop, dentry);
> > > 	if (err)
> > > 		return ERR_PTR(err);  
> > 
> > But if the dir creation fails, there is no need to allocate devm
> > destructor...  
> 
> Yes, but why do we care about that?
> debugfs is designed the way that we should ignore errors from it.
> 
> This discussion let me think, that probably the best solution is to add a field
> to struct device_driver to handle this.
> 
> debugfs will appear only when device is completely instantiated, removed before
> anything, and a lot of other benefits (see the story behind dev_groups as a
> reference).
> 

Okay, this seems way beyond the scope of what I am trying to do here.

Since Matti is also not entirely content with these additions to the
devm-helpers.h header and you suggested to use gpiod_to_irq(), I shall
drop these changes from the next version and just use
devm_add_action_or_reset() in the driver directly.

Marek

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 07/11] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG
  2024-04-23 16:43       ` Andy Shevchenko
@ 2024-04-23 16:57         ` Marek Behún
  2024-04-23 17:25           ` Andy Shevchenko
  0 siblings, 1 reply; 36+ messages in thread
From: Marek Behún @ 2024-04-23 16:57 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andy Shevchenko, Gregory CLEMENT, Arnd Bergmann, soc,
	Olivia Mackall, Herbert Xu, Greg Kroah-Hartman, linux-crypto,
	arm

On Tue, 23 Apr 2024 19:43:41 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Tue, Apr 23, 2024 at 7:32 PM Marek Behún <kabel@kernel.org> wrote:
> > On Tue, 23 Apr 2024 18:58:19 +0300
> > Andy Shevchenko <andy@kernel.org> wrote:  
> > > On Thu, Apr 18, 2024 at 02:11:12PM +0200, Marek Behún wrote:  
> 
> ...
> 
> > > > +   irq_idx = omnia_int_to_gpio_idx[__bf_shf(INT_TRNG)];
> > > > +   irq = devm_irq_create_mapping(dev, mcu->gc.irq.domain, irq_idx);
> > > > +   if (irq < 0)
> > > > +           return dev_err_probe(dev, irq, "Cannot map TRNG IRQ\n");  
> > >
> > > This looks like some workaround against existing gpiod_to_irq(). Why do you
> > > need this?  
> >
> > Hmmm, I thought that would not work because that line is only valid
> > as an IRQ, not as a GPIO (this is enforced via the valid_mask member of
> > gpio_chip and gpio_irq_chip).
> >
> > But looking at the code of gpiolib, if I do
> >   irq = gpiod_to_irq(gpiochip_get_desc(gc, irq_idx));
> > the valid_mask is not enforced anywhere.  
> 
> Which one? GPIO has two: one per GPIO realm and one for IRQ domain.

The GPIO line validity is not enforced. The IRQ line validity is
enforced in the gpiochip_to_irq() method.

> > Is this semantically right to do even in spite of the fact that the
> > line is not a valid GPIO line?  
> 
> Yes. It's orthogonal to that.
> 


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 07/11] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG
  2024-04-23 16:57         ` Marek Behún
@ 2024-04-23 17:25           ` Andy Shevchenko
  2024-04-24 16:55             ` Marek Behún
  0 siblings, 1 reply; 36+ messages in thread
From: Andy Shevchenko @ 2024-04-23 17:25 UTC (permalink / raw)
  To: Marek Behún
  Cc: Gregory CLEMENT, Arnd Bergmann, soc, Olivia Mackall, Herbert Xu,
	Greg Kroah-Hartman, linux-crypto, arm

On Tue, Apr 23, 2024 at 06:57:04PM +0200, Marek Behún wrote:
> On Tue, 23 Apr 2024 19:43:41 +0300
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Tue, Apr 23, 2024 at 7:32 PM Marek Behún <kabel@kernel.org> wrote:
> > > On Tue, 23 Apr 2024 18:58:19 +0300
> > > Andy Shevchenko <andy@kernel.org> wrote:  
> > > > On Thu, Apr 18, 2024 at 02:11:12PM +0200, Marek Behún wrote:  

...

> > > > > +   irq_idx = omnia_int_to_gpio_idx[__bf_shf(INT_TRNG)];
> > > > > +   irq = devm_irq_create_mapping(dev, mcu->gc.irq.domain, irq_idx);
> > > > > +   if (irq < 0)
> > > > > +           return dev_err_probe(dev, irq, "Cannot map TRNG IRQ\n");  
> > > >
> > > > This looks like some workaround against existing gpiod_to_irq(). Why do you
> > > > need this?  
> > >
> > > Hmmm, I thought that would not work because that line is only valid
> > > as an IRQ, not as a GPIO (this is enforced via the valid_mask member of
> > > gpio_chip and gpio_irq_chip).
> > >
> > > But looking at the code of gpiolib, if I do
> > >   irq = gpiod_to_irq(gpiochip_get_desc(gc, irq_idx));
> > > the valid_mask is not enforced anywhere.  
> > 
> > Which one? GPIO has two: one per GPIO realm and one for IRQ domain.
> 
> The GPIO line validity is not enforced. The IRQ line validity is
> enforced in the gpiochip_to_irq() method.

Okay, but does it work for you as expected then?

If not, we should fix GPIO library to have gpiod_to_irq() to work as expected.

> > > Is this semantically right to do even in spite of the fact that the
> > > line is not a valid GPIO line?  
> > 
> > Yes. It's orthogonal to that.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v6 07/11] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG
  2024-04-23 17:25           ` Andy Shevchenko
@ 2024-04-24 16:55             ` Marek Behún
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Behún @ 2024-04-24 16:55 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Gregory CLEMENT, Arnd Bergmann, soc, Olivia Mackall, Herbert Xu,
	Greg Kroah-Hartman, linux-crypto, arm

On Tue, 23 Apr 2024 20:25:14 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Tue, Apr 23, 2024 at 06:57:04PM +0200, Marek Behún wrote:
> > On Tue, 23 Apr 2024 19:43:41 +0300
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:  
> > > On Tue, Apr 23, 2024 at 7:32 PM Marek Behún <kabel@kernel.org> wrote:  
> > > > On Tue, 23 Apr 2024 18:58:19 +0300
> > > > Andy Shevchenko <andy@kernel.org> wrote:    
> > > > > On Thu, Apr 18, 2024 at 02:11:12PM +0200, Marek Behún wrote:    
> 
> ...
> 
> > > > > > +   irq_idx = omnia_int_to_gpio_idx[__bf_shf(INT_TRNG)];
> > > > > > +   irq = devm_irq_create_mapping(dev, mcu->gc.irq.domain, irq_idx);
> > > > > > +   if (irq < 0)
> > > > > > +           return dev_err_probe(dev, irq, "Cannot map TRNG IRQ\n");    
> > > > >
> > > > > This looks like some workaround against existing gpiod_to_irq(). Why do you
> > > > > need this?    
> > > >
> > > > Hmmm, I thought that would not work because that line is only valid
> > > > as an IRQ, not as a GPIO (this is enforced via the valid_mask member of
> > > > gpio_chip and gpio_irq_chip).
> > > >
> > > > But looking at the code of gpiolib, if I do
> > > >   irq = gpiod_to_irq(gpiochip_get_desc(gc, irq_idx));
> > > > the valid_mask is not enforced anywhere.    
> > > 
> > > Which one? GPIO has two: one per GPIO realm and one for IRQ domain.  
> > 
> > The GPIO line validity is not enforced. The IRQ line validity is
> > enforced in the gpiochip_to_irq() method.  
> 
> Okay, but does it work for you as expected then?
> 
> If not, we should fix GPIO library to have gpiod_to_irq() to work as expected.

Yes, it does. I am going to send a new version in a few minutes.

Marek

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2024-04-24 16:55 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-18 12:11 [PATCH v6 00/11] Turris Omnia MCU driver Marek Behún
2024-04-18 12:11 ` [PATCH v6 01/11] dt-bindings: arm: add cznic,turris-omnia-mcu binding Marek Behún
2024-04-18 15:43   ` Conor Dooley
2024-04-19  8:23     ` Marek Behún
2024-04-19 11:14       ` Marek Behún
2024-04-19 13:52         ` Conor Dooley
2024-04-18 12:11 ` [PATCH v6 02/11] platform: cznic: Add preliminary support for Turris Omnia MCU Marek Behún
2024-04-18 12:11 ` [PATCH v6 03/11] platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs Marek Behún
2024-04-18 12:11 ` [PATCH v6 04/11] platform: cznic: turris-omnia-mcu: Add support for poweroff and wakeup Marek Behún
2024-04-18 12:11 ` [PATCH v6 05/11] platform: cznic: turris-omnia-mcu: Add support for MCU watchdog Marek Behún
2024-04-18 13:26   ` Guenter Roeck
2024-04-18 14:15     ` Marek Behún
2024-04-18 12:11 ` [PATCH v6 06/11] devm-helpers: Add resource managed version of irq_create_mapping() Marek Behún
2024-04-23 12:32   ` Matti Vaittinen
2024-04-23 12:46   ` Andy Shevchenko
2024-04-23 14:21     ` Marek Behún
2024-04-23 15:14     ` Marek Behún
2024-04-23 15:56       ` Andy Shevchenko
2024-04-18 12:11 ` [PATCH v6 07/11] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG Marek Behún
2024-04-23 15:58   ` Andy Shevchenko
2024-04-23 16:32     ` Marek Behún
2024-04-23 16:43       ` Andy Shevchenko
2024-04-23 16:57         ` Marek Behún
2024-04-23 17:25           ` Andy Shevchenko
2024-04-24 16:55             ` Marek Behún
2024-04-18 12:11 ` [PATCH v6 08/11] devm-helpers: Add resource managed version of debugfs directory create function Marek Behún
2024-04-23 12:33   ` Matti Vaittinen
2024-04-23 16:02   ` Andy Shevchenko
2024-04-23 16:20     ` Marek Behún
2024-04-23 16:33       ` Andy Shevchenko
2024-04-23 16:43         ` Marek Behún
2024-04-23 16:21     ` Dan Carpenter
2024-04-18 12:11 ` [PATCH v6 09/11] platform: cznic: turris-omnia-mcu: Add support for digital message signing via debugfs Marek Behún
2024-04-18 12:11 ` [PATCH v6 10/11] ARM: dts: turris-omnia: Add MCU system-controller node Marek Behún
2024-04-18 12:11 ` [PATCH v6 11/11] ARM: dts: turris-omnia: Add GPIO key node for front button Marek Behún
2024-04-23 16:05 ` [PATCH v6 00/11] Turris Omnia MCU driver Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).