[PATCH v7 0/9] Turris Omnia MCU driver

[PATCH v7 0/9] Turris Omnia MCU driver

[Marek Behún]

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

this is v7 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 also cover letters for v1, v2, v3, v4, v5 and v6:
  https://patchwork.kernel.org/project/linux-soc/cover/20230823161012.6986-1-kabel@kernel.org/
  https://patchwork.kernel.org/project/linux-soc/cover/20230919103815.16818-1-kabel@kernel.org/
  https://patchwork.kernel.org/project/linux-soc/cover/20231023143130.11602-1-kabel@kernel.org/
  https://patchwork.kernel.org/project/linux-soc/cover/20231026161803.16750-1-kabel@kernel.org/
  https://patchwork.kernel.org/project/linux-soc/cover/20240323164359.21642-1-kabel@kernel.org/
  https://patchwork.kernel.org/project/linux-soc/cover/20240418121116.22184-1-kabel@kernel.org/

Changes since v6:
- moved the DT binding from arm/cznic,turris-omnia-mcu.yaml to
  firmware/cznic,turris-omnia-mcu.yaml, as suggested by Conor Dooley
  (patch 1)
- dropped the devm-helpers.h additions, for the reasons see
  https://lore.kernel.org/soc/20240423184346.37eb0915@thinkpad/
- use gpiod_to_irq(gpiochip_get_desc(...)) instead of
  irq_create_mapping(), as suggested by Andy Shevchenko (patches 6 and
  7)
- added a dummy read of TRNG entropy when registering TRNG, in case
  someone cleared the TRNG interrupt before probing the driver, but did
  not read the entropy (the MCU won't send a new TRNG interrupt if the
  entropy is not collected) (patch 6)
- fixed a bug in TRNG probing, wherein if the 

Marek Behún (9):
  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
  platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG
  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 ++
 .../firmware/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 |  216 ++++
 .../platform/cznic/turris-omnia-mcu-gpio.c    | 1047 +++++++++++++++++
 .../cznic/turris-omnia-mcu-sys-off-wakeup.c   |  258 ++++
 .../platform/cznic/turris-omnia-mcu-trng.c    |  109 ++
 .../cznic/turris-omnia-mcu-watchdog.c         |  123 ++
 drivers/platform/cznic/turris-omnia-mcu.h     |  188 +++
 include/linux/turris-omnia-mcu-interface.h    |  249 ++++
 17 files changed, 2956 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/firmware/cznic,turris-omnia-mcu.yaml
 create mode 100644 drivers/platform/cznic/Kconfig
 create mode 100644 drivers/platform/cznic/Makefile
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-base.c
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-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

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

[Marek Behún]

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

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/platform/cznic/Kconfig                |   2 +
 drivers/platform/cznic/Makefile               |   1 +
 .../platform/cznic/turris-omnia-mcu-base.c    |   6 +-
 .../platform/cznic/turris-omnia-mcu-gpio.c    |   2 +-
 .../platform/cznic/turris-omnia-mcu-trng.c    | 109 ++++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |   8 ++
 6 files changed, 126 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..a9ac36b570e9
--- /dev/null
+++ b/drivers/platform/cznic/turris-omnia-mcu-trng.c
@@ -0,0 +1,109 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CZ.NIC's Turris Omnia MCU TRNG driver
+ *
+ * 2024 by Marek Behún <kabel@kernel.org>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/completion.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+#include <linux/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;
+}
+
+static void omnia_irq_mapping_drop(void *res)
+{
+	irq_dispose_mapping((unsigned int)(unsigned long)res);
+}
+
+int omnia_mcu_register_trng(struct omnia_mcu *mcu)
+{
+	struct device *dev = &mcu->client->dev;
+	u8 irq_idx, dummy;
+	int irq, err;
+
+	if (!(mcu->features & FEAT_TRNG))
+		return 0;
+
+	irq_idx = omnia_int_to_gpio_idx[__bf_shf(INT_TRNG)];
+	irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
+	if (irq < 0)
+		return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");
+
+	err = devm_add_action_or_reset(dev, omnia_irq_mapping_drop,
+				       (void *)(unsigned long)irq);
+	if (err)
+		return err;
+
+	/* If someone else cleared the TRNG interrupt but did not read the
+	 * entropy, a new interrupt won't be generated, and entropy collection
+	 * will be stuck. Ensure an interrupt will be generated by executing
+	 * the collect entropy command (and discarding the result).
+	 */
+	err = omnia_cmd_read(mcu->client, CMD_TRNG_COLLECT_ENTROPY, &dummy, 1);
+	if (err)
+		return err;
+
+	init_completion(&mcu->trng_completion);
+
+	err = devm_request_threaded_irq(dev, irq, NULL, omnia_trng_irq_handler,
+					IRQF_ONESHOT, "turris-omnia-mcu-trng",
+					mcu);
+	if (err)
+		return dev_err_probe(dev, err, "Cannot request TRNG IRQ\n");
+
+	mcu->trng.name = "turris-omnia-mcu-trng";
+	mcu->trng.read = omnia_trng_read;
+	mcu->trng.priv = (unsigned long)mcu;
+
+	err = devm_hwrng_register(dev, &mcu->trng);
+	if (err)
+		return dev_err_probe(dev, err, "Cannot register TRNG\n");
+
+	return 0;
+}
diff --git a/drivers/platform/cznic/turris-omnia-mcu.h b/drivers/platform/cznic/turris-omnia-mcu.h
index 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

[PATCH v7 7/9] platform: cznic: turris-omnia-mcu: Add support for digital message signing via debugfs

[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 | 216 ++++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |  23 ++
 8 files changed, 313 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 513b0cd75590..8a707cd637a6 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..585a59bb5e2e
--- /dev/null
+++ b/drivers/platform/cznic/turris-omnia-mcu-debugfs.c
@@ -0,0 +1,216 @@
+// 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/gpio/consumer.h>
+#include <linux/gpio/driver.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,
+};
+
+static void omnia_irq_mapping_drop(void *res)
+{
+	irq_dispose_mapping((unsigned int)(unsigned long)res);
+}
+
+static void omnia_debugfs_drop_dir(void *res)
+{
+	debugfs_remove_recursive(res);
+}
+
+int omnia_mcu_register_debugfs(struct omnia_mcu *mcu)
+{
+	struct device *dev = &mcu->client->dev;
+	struct dentry *root;
+	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 = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
+	if (irq < 0)
+		return dev_err_probe(dev, irq,
+				     "Cannot get MESSAGE_SIGNED IRQ\n");
+
+	err = devm_add_action_or_reset(dev, omnia_irq_mapping_drop,
+				       (void *)(unsigned long)irq);
+	if (err)
+		return err;
+
+	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 = debugfs_create_dir("turris-omnia-mcu", NULL);
+	debugfs_create_file_unsafe("do_sign", 0600, root, mcu, &do_sign_fops);
+
+	return devm_add_action_or_reset(dev, omnia_debugfs_drop_dir, root);
+}
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

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

[Andy Shevchenko]

On Wed, Apr 24, 2024 at 07:38:05PM +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.

...

> +static void omnia_irq_mapping_drop(void *res)
> +{
> +	irq_dispose_mapping((unsigned int)(unsigned long)res);
> +}

Leftover?

> +int omnia_mcu_register_trng(struct omnia_mcu *mcu)
> +{
> +	struct device *dev = &mcu->client->dev;
> +	u8 irq_idx, dummy;
> +	int irq, err;
> +
> +	if (!(mcu->features & FEAT_TRNG))
> +		return 0;
> +
> +	irq_idx = omnia_int_to_gpio_idx[__bf_shf(INT_TRNG)];
> +	irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
> +	if (irq < 0)
> +		return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");

> +	err = devm_add_action_or_reset(dev, omnia_irq_mapping_drop,
> +				       (void *)(unsigned long)irq);
> +	if (err)
> +		return err;

Are you sure it's correct now?

> +	/* If someone else cleared the TRNG interrupt but did not read the
> +	 * entropy, a new interrupt won't be generated, and entropy collection
> +	 * will be stuck. Ensure an interrupt will be generated by executing
> +	 * the collect entropy command (and discarding the result).
> +	 */
> +	err = omnia_cmd_read(mcu->client, CMD_TRNG_COLLECT_ENTROPY, &dummy, 1);
> +	if (err)
> +		return err;
> +
> +	init_completion(&mcu->trng_completion);
> +
> +	err = devm_request_threaded_irq(dev, irq, NULL, omnia_trng_irq_handler,
> +					IRQF_ONESHOT, "turris-omnia-mcu-trng",
> +					mcu);
> +	if (err)
> +		return dev_err_probe(dev, err, "Cannot request TRNG IRQ\n");
> +
> +	mcu->trng.name = "turris-omnia-mcu-trng";
> +	mcu->trng.read = omnia_trng_read;
> +	mcu->trng.priv = (unsigned long)mcu;
> +
> +	err = devm_hwrng_register(dev, &mcu->trng);
> +	if (err)
> +		return dev_err_probe(dev, err, "Cannot register TRNG\n");
> +
> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Wed, Apr 24, 2024 at 07:38:05PM +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.

...

> +	/* If someone else cleared the TRNG interrupt but did not read the
> +	 * entropy, a new interrupt won't be generated, and entropy collection
> +	 * will be stuck. Ensure an interrupt will be generated by executing
> +	 * the collect entropy command (and discarding the result).
> +	 */

/*
 * Note, the above style is solely for net subsystem. The
 * others should use the usual one, like in this example.
 */

-- 
With Best Regards,
Andy Shevchenko

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

[Marek Behún]

On Wed, 24 Apr 2024 21:33:44 +0300
Andy Shevchenko <andy@kernel.org> wrote:

> On Wed, Apr 24, 2024 at 07:38:05PM +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.  
> 
> ...
> 
> > +static void omnia_irq_mapping_drop(void *res)
> > +{
> > +	irq_dispose_mapping((unsigned int)(unsigned long)res);
> > +}  
> 
> Leftover?

What do you mean? I dropped the devm-helpers.h changes, now I do
devm_add_action_or_reset() manually, with this function as the action.

> > +int omnia_mcu_register_trng(struct omnia_mcu *mcu)
> > +{
> > +	struct device *dev = &mcu->client->dev;
> > +	u8 irq_idx, dummy;
> > +	int irq, err;
> > +
> > +	if (!(mcu->features & FEAT_TRNG))
> > +		return 0;
> > +
> > +	irq_idx = omnia_int_to_gpio_idx[__bf_shf(INT_TRNG)];
> > +	irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
> > +	if (irq < 0)
> > +		return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");  
> 
> > +	err = devm_add_action_or_reset(dev, omnia_irq_mapping_drop,
> > +				       (void *)(unsigned long)irq);
> > +	if (err)
> > +		return err;  
> 
> Are you sure it's correct now?

Yes, why wouldn't it?

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

[Andy Shevchenko]

On Wed, Apr 24, 2024 at 08:51:23PM +0200, Marek Behún wrote:
> On Wed, 24 Apr 2024 21:33:44 +0300
> Andy Shevchenko <andy@kernel.org> wrote:
> > On Wed, Apr 24, 2024 at 07:38:05PM +0200, Marek Behún wrote:

...

> > > +static void omnia_irq_mapping_drop(void *res)
> > > +{
> > > +	irq_dispose_mapping((unsigned int)(unsigned long)res);
> > > +}
> > 
> > Leftover?
> 
> What do you mean? I dropped the devm-helpers.h changes, now I do
> devm_add_action_or_reset() manually, with this function as the action.

But why?

...

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

> > > +	err = devm_add_action_or_reset(dev, omnia_irq_mapping_drop,
> > > +				       (void *)(unsigned long)irq);
> > > +	if (err)
> > > +		return err;
> > 
> > Are you sure it's correct now?
> 
> Yes, why wouldn't it?

For what purpose? I don't see drivers doing that. Are you expecting that
the same IRQ mapping will be reused for something else? Can you elaborate
how? (I can imagine one theoretical / weird case how to achieve that,
but impractical.)

Besides above, this is asymmetrical call to gpiod_to_irq(). If we really care
about this, it should be provided by GPIO library.

-- 
With Best Regards,
Andy Shevchenko

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

[Marek Behún]

On Wed, 24 Apr 2024 22:47:10 +0300
Andy Shevchenko <andy@kernel.org> wrote:

> On Wed, Apr 24, 2024 at 08:51:23PM +0200, Marek Behún wrote:
> > On Wed, 24 Apr 2024 21:33:44 +0300
> > Andy Shevchenko <andy@kernel.org> wrote:  
> > > On Wed, Apr 24, 2024 at 07:38:05PM +0200, Marek Behún wrote:  
> 
> ...
> 
> > > > +static void omnia_irq_mapping_drop(void *res)
> > > > +{
> > > > +	irq_dispose_mapping((unsigned int)(unsigned long)res);
> > > > +}  
> > > 
> > > Leftover?  
> > 
> > What do you mean? I dropped the devm-helpers.h changes, now I do
> > devm_add_action_or_reset() manually, with this function as the action.  
> 
> But why?
> 
> ...
> 
> > > > +	irq_idx = omnia_int_to_gpio_idx[__bf_shf(INT_TRNG)];
> > > > +	irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
> > > > +	if (irq < 0)
> > > > +		return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");  
> 
> > > > +	err = devm_add_action_or_reset(dev, omnia_irq_mapping_drop,
> > > > +				       (void *)(unsigned long)irq);
> > > > +	if (err)
> > > > +		return err;  
> > > 
> > > Are you sure it's correct now?  
> > 
> > Yes, why wouldn't it?  
> 
> For what purpose? I don't see drivers doing that. Are you expecting that
> the same IRQ mapping will be reused for something else? Can you elaborate
> how? (I can imagine one theoretical / weird case how to achieve that,
> but impractical.)

I do a lot of binding/unbinding of that driver. I was under the
impression that all resources should be dropped on driver unbind.

> Besides above, this is asymmetrical call to gpiod_to_irq(). If we really care
> about this, it should be provided by GPIO library.
> 

Something like the following?

From 5aac93d55f6fb750726f7e879672142956981a4c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Beh=C3=BAn?= <kabel@kernel.org>
Date: Thu, 25 Apr 2024 11:33:33 +0200
Subject: [PATCH] gpiolib: devres: Add resource managed version of
 gpiod_to_irq()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Add devm_gpiod_to_irq(), a resource managed version of gpiod_to_irq().
The release function calls irq_dispose_mapping().

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/gpio/gpiolib-devres.c | 27 +++++++++++++++++++++++++++
 include/linux/gpio/consumer.h | 10 ++++++++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/gpio/gpiolib-devres.c b/drivers/gpio/gpiolib-devres.c
index 4987e62dcb3d..98a40492e596 100644
--- a/drivers/gpio/gpiolib-devres.c
+++ b/drivers/gpio/gpiolib-devres.c
@@ -12,6 +12,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/device.h>
 #include <linux/gfp.h>
+#include <linux/irqdomain.h>
 
 #include "gpiolib.h"
 
@@ -427,3 +428,29 @@ int devm_gpiochip_add_data_with_key(struct device *dev, struct gpio_chip *gc, vo
 	return devm_add_action_or_reset(dev, devm_gpio_chip_release, gc);
 }
 EXPORT_SYMBOL_GPL(devm_gpiochip_add_data_with_key);
+
+static void devm_gpiod_irq_release(void *data)
+{
+	irq_dispose_mapping((unsigned int)(unsigned long)data);
+}
+
+/**
+ * devm_gpiod_to_irq() - Resource managed devm_gpiod_to_irq()
+ * @dev: pointer to the device that gpio_chip belongs to.
+ * @desc: gpio whose IRQ will be returned
+ *
+ * Return the IRQ corresponding to the passed GPIO, or an error code in case of
+ * error.
+ */
+int devm_gpiod_to_irq(struct device *dev, const struct gpio_desc *desc)
+{
+	int virq;
+
+	virq = gpiod_to_irq(desc);
+	if (virq < 0)
+		return virq;
+
+	return devm_add_action_or_reset(dev, devm_gpiod_irq_release,
+					(void *)(unsigned long)virq);
+}
+EXPORT_SYMBOL_GPL(devm_gpiod_to_irq);
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index db2dfbae8edb..e8f4829538f6 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -165,6 +165,8 @@ int gpiod_is_active_low(const struct gpio_desc *desc);
 int gpiod_cansleep(const struct gpio_desc *desc);
 
 int gpiod_to_irq(const struct gpio_desc *desc);
+int devm_gpiod_to_irq(struct device *dev, const struct gpio_desc *desc);
+
 int gpiod_set_consumer_name(struct gpio_desc *desc, const char *name);
 
 /* Convert between the old gpio_ and new gpiod_ interfaces */
@@ -519,6 +521,14 @@ static inline int gpiod_to_irq(const struct gpio_desc *desc)
 	return -EINVAL;
 }
 
+static inline int devm_gpiod_to_irq(struct device *dev,
+				    const struct gpio_desc *desc)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(desc);
+	return -EINVAL;
+}
+
 static inline int gpiod_set_consumer_name(struct gpio_desc *desc,
 					  const char *name)
 {
-- 
2.43.2

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

[Andy Shevchenko]

On Wed, Apr 24, 2024 at 10:47:11PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 24, 2024 at 08:51:23PM +0200, Marek Behún wrote:
> > On Wed, 24 Apr 2024 21:33:44 +0300
> > Andy Shevchenko <andy@kernel.org> wrote:
> > > On Wed, Apr 24, 2024 at 07:38:05PM +0200, Marek Behún wrote:

...

> > > > +static void omnia_irq_mapping_drop(void *res)
> > > > +{
> > > > +	irq_dispose_mapping((unsigned int)(unsigned long)res);
> > > > +}
> > > 
> > > Leftover?
> > 
> > What do you mean? I dropped the devm-helpers.h changes, now I do
> > devm_add_action_or_reset() manually, with this function as the action.
> 
> But why?

...

> > > > +	irq_idx = omnia_int_to_gpio_idx[__bf_shf(INT_TRNG)];
> > > > +	irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
> > > > +	if (irq < 0)
> > > > +		return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");
> 
> > > > +	err = devm_add_action_or_reset(dev, omnia_irq_mapping_drop,
> > > > +				       (void *)(unsigned long)irq);
> > > > +	if (err)
> > > > +		return err;
> > > 
> > > Are you sure it's correct now?
> > 
> > Yes, why wouldn't it?
> 
> For what purpose? I don't see drivers doing that. Are you expecting that
> the same IRQ mapping will be reused for something else? Can you elaborate
> how? (I can imagine one theoretical / weird case how to achieve that,
> but impractical.)
> 
> Besides above, this is asymmetrical call to gpiod_to_irq(). If we really care
> about this, it should be provided by GPIO library.

FWIW, gpiochip_irqchip_remove() disposes mappings for internally allocated IRQ
domains. So with your code it even might be a double-disposal.

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Thu, Apr 25, 2024 at 11:34:47AM +0200, Marek Behún wrote:
> On Wed, 24 Apr 2024 22:47:10 +0300
> Andy Shevchenko <andy@kernel.org> wrote:
> > On Wed, Apr 24, 2024 at 08:51:23PM +0200, Marek Behún wrote:

...

> > For what purpose? I don't see drivers doing that. Are you expecting that
> > the same IRQ mapping will be reused for something else? Can you elaborate
> > how? (I can imagine one theoretical / weird case how to achieve that,
> > but impractical.)
> 
> I do a lot of binding/unbinding of that driver. I was under the
> impression that all resources should be dropped on driver unbind.
> 
> > Besides above, this is asymmetrical call to gpiod_to_irq(). If we really care
> > about this, it should be provided by GPIO library.
> 
> Something like the following?

Not needed. IRQ mappings are per domain, and GPIO chip has its own associated
with the respective lifetime, AFAIU when you remove the GPIO chip, all mappings
will be disposed (as I pointed out in previous mail).

-- 
With Best Regards,
Andy Shevchenko

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

[Marek Behún]

On Thu, 25 Apr 2024 13:04:07 +0300
Andy Shevchenko <andy@kernel.org> wrote:

> On Thu, Apr 25, 2024 at 11:34:47AM +0200, Marek Behún wrote:
> > On Wed, 24 Apr 2024 22:47:10 +0300
> > Andy Shevchenko <andy@kernel.org> wrote:  
> > > On Wed, Apr 24, 2024 at 08:51:23PM +0200, Marek Behún wrote:  
> 
> ...
> 
> > > For what purpose? I don't see drivers doing that. Are you expecting that
> > > the same IRQ mapping will be reused for something else? Can you elaborate
> > > how? (I can imagine one theoretical / weird case how to achieve that,
> > > but impractical.)  
> > 
> > I do a lot of binding/unbinding of that driver. I was under the
> > impression that all resources should be dropped on driver unbind.
> >   
> > > Besides above, this is asymmetrical call to gpiod_to_irq(). If we really care
> > > about this, it should be provided by GPIO library.  
> > 
> > Something like the following?  
> 
> Not needed. IRQ mappings are per domain, and GPIO chip has its own associated
> with the respective lifetime, AFAIU when you remove the GPIO chip, all mappings
> will be disposed (as I pointed out in previous mail).
> 

OMG you are right :) of course. OK, I shall drop this.

Marek

Re: [PATCH v7 0/9] Turris Omnia MCU driver

[Gregory CLEMENT]

Hello all,

> Hello Andy, Hans, Ilpo, Arnd, Gregory, and others,
>
> this is v7 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 also cover letters for v1, v2, v3, v4, v5 and v6:
>   https://patchwork.kernel.org/project/linux-soc/cover/20230823161012.6986-1-kabel@kernel.org/
>   https://patchwork.kernel.org/project/linux-soc/cover/20230919103815.16818-1-kabel@kernel.org/
>   https://patchwork.kernel.org/project/linux-soc/cover/20231023143130.11602-1-kabel@kernel.org/
>   https://patchwork.kernel.org/project/linux-soc/cover/20231026161803.16750-1-kabel@kernel.org/
>   https://patchwork.kernel.org/project/linux-soc/cover/20240323164359.21642-1-kabel@kernel.org/
>   https://patchwork.kernel.org/project/linux-soc/cover/20240418121116.22184-1-kabel@kernel.org/
>
> Changes since v6:
> - moved the DT binding from arm/cznic,turris-omnia-mcu.yaml to
>   firmware/cznic,turris-omnia-mcu.yaml, as suggested by Conor Dooley
>   (patch 1)
> - dropped the devm-helpers.h additions, for the reasons see
>   https://lore.kernel.org/soc/20240423184346.37eb0915@thinkpad/
> - use gpiod_to_irq(gpiochip_get_desc(...)) instead of
>   irq_create_mapping(), as suggested by Andy Shevchenko (patches 6 and
>   7)
> - added a dummy read of TRNG entropy when registering TRNG, in case
>   someone cleared the TRNG interrupt before probing the driver, but did
>   not read the entropy (the MCU won't send a new TRNG interrupt if the
>   entropy is not collected) (patch 6)
> - fixed a bug in TRNG probing, wherein if the 
>
> Marek Behún (9):
>   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
>   platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG
>   platform: cznic: turris-omnia-mcu: Add support for digital message
>     signing via debugfs

It is still early as there are some comment pending but I wonder who
will responsible of merging all theses patches ?

Arnd ? Hans ? Ilpo ? me ?

Gregory


>   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 ++
>  .../firmware/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 |  216 ++++
>  .../platform/cznic/turris-omnia-mcu-gpio.c    | 1047 +++++++++++++++++
>  .../cznic/turris-omnia-mcu-sys-off-wakeup.c   |  258 ++++
>  .../platform/cznic/turris-omnia-mcu-trng.c    |  109 ++
>  .../cznic/turris-omnia-mcu-watchdog.c         |  123 ++
>  drivers/platform/cznic/turris-omnia-mcu.h     |  188 +++
>  include/linux/turris-omnia-mcu-interface.h    |  249 ++++
>  17 files changed, 2956 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/firmware/cznic,turris-omnia-mcu.yaml
>  create mode 100644 drivers/platform/cznic/Kconfig
>  create mode 100644 drivers/platform/cznic/Makefile
>  create mode 100644 drivers/platform/cznic/turris-omnia-mcu-base.c
>  create mode 100644 drivers/platform/cznic/turris-omnia-mcu-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

Re: [PATCH v7 0/9] Turris Omnia MCU driver

[Marek Behún]

On Fri, 26 Apr 2024 18:13:32 +0200
Gregory CLEMENT <gregory.clement@bootlin.com> wrote:

> Hello all,
>
...
> 
> It is still early as there are some comment pending but I wonder who
> will responsible of merging all theses patches ?
> 
> Arnd ? Hans ? Ilpo ? me ?

I am assigning these patches to Arnd on patchwork all this time, so I
guess it could be him, unless he has a problem with this?

I've just sent v8.

Marek