Turris Omnia MCU driver

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 <[email protected]>

Reviewed-by Krzysztof Kozlowski <[email protected]>

---
 .../firmware/cznic,turris-omnia-mcu.yaml      | 86 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 87 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/cznic,turris-omnia-mcu.yaml

Quoted 4 lines · Marek Behún

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

Quoted 8 lines

Signed-off-by Marek Behún <[email protected]>

Reviewed-by Krzysztof Kozlowski <[email protected]>

---
 .../firmware/cznic,turris-omnia-mcu.yaml      | 86 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 87 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/cznic,turris-omnia-mcu.yaml

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/firmware/cznic,turris-omnia-mcu.yaml: $id: Cannot determine base path from $id, relative path/filename doesn't match actual path or filename

(quoted diff) +0 -0

 	 $id: http://devicetree.org/schemas/arm/cznic,turris-omnia-mcu.yaml
 	file: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/firmware/cznic,turris-omnia-mcu.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.

Quoted 34 lines · On Wed, 24 Apr 2024 13:16:18 -0500 Rob Herring <[email protected]> wrote:

Quoted 4 lines · Marek Behún

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

Quoted 8 lines

Signed-off-by Marek Behún <[email protected]>

Reviewed-by Krzysztof Kozlowski <[email protected]>

---
 .../firmware/cznic,turris-omnia-mcu.yaml      | 86 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 87 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/cznic,turris-omnia-mcu.yaml

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/firmware/cznic,turris-omnia-mcu.yaml: $id: Cannot determine base path from $id, relative path/filename doesn't match actual path or filename

(quoted diff) +0 -0

 	 $id: http://devicetree.org/schemas/arm/cznic,turris-omnia-mcu.yaml
 	file: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/firmware/cznic,turris-omnia-mcu.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

Quoted 6 lines

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.

Sigh. I moved the patch bud did not change it in $id...

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 <[email protected]>

---
 .../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

Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu +16 -0

index b27b60d00c87..564119849388 100644
@@ -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 <[email protected]>
+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

drivers/platform/cznic/Kconfig +4 -0

index 3a8c3edcd7e6..0a752aa654fa 100644
@@ -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

drivers/platform/cznic/Makefile +1 -0

index 53fd8f1777a3..a185ef882e44 100644
@@ -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

drivers/platform/cznic/turris-omnia-mcu-base.c +5 -0

index 708b084cb5da..697ace090bda 100644
@@ -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);
 }
 

drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c new file +258 -0

new file mode 100644
index 000000000000..fbd1b4cd7011
@@ -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 <[email protected]>
+ */
+
+#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;
+}

drivers/platform/cznic/turris-omnia-mcu.h +20 -0

index e8d9bf1c8fdc..6179a4f2069e 100644
@@ -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 */

Signature · 1 line

2.43.2

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.

drivers/platform/cznic/turris-omnia-mcu-trng.c new file +8 -0

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

drivers/platform/cznic/turris-omnia-mcu-trng.c new file +27 -0

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

Quoted 4 lines · 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.

...

drivers/platform/cznic/turris-omnia-mcu-trng.c line 62

Quoted 4 lines

drivers/platform/cznic/turris-omnia-mcu-trng.c new file +4 -0

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

Leftover?

Quoted 13 lines

drivers/platform/cznic/turris-omnia-mcu-trng.c new file +13 -0

+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");  

drivers/platform/cznic/turris-omnia-mcu-trng.c line 81

Quoted 4 lines

drivers/platform/cznic/turris-omnia-mcu-trng.c new file +4 -0

+	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?

Quoted 1 line · On Wed, 24 Apr 2024 21:33:44 +0300 Andy Shevchenko <[email protected]> wrote:

On Wed, Apr 24, 2024 at 07:38:05PM +0200, Marek Behún wrote:

Quoted 6 lines

drivers/platform/cznic/turris-omnia-mcu-trng.c line 62

Quoted 4 lines

drivers/platform/cznic/turris-omnia-mcu-trng.c new file +4 -0

+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.

Quoted 4 lines

Quoted 4 lines

drivers/platform/cznic/turris-omnia-mcu-trng.c new file +4 -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");

Quoted 6 lines

drivers/platform/cznic/turris-omnia-mcu-trng.c line 81

Quoted 4 lines

drivers/platform/cznic/turris-omnia-mcu-trng.c new file +4 -0

+	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?

Quoted 3 lines · Marek Behún

Quoted 1 line · On Wed, 24 Apr 2024 21:33:44 +0300 Andy Shevchenko <[email protected]> wrote:

On Wed, Apr 24, 2024 at 07:38:05PM +0200, Marek Behún wrote:

...

Quoted 9 lines

Quoted 6 lines

Review

Quoted 4 lines

(quoted diff) +4 -0

+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?

...

Quoted 12 lines

Quoted 11 lines

Review

Quoted 9 lines

(quoted diff) +8 -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?

Quoted 1 line

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.

Quoted 1 line · On Wed, 24 Apr 2024 22:47:10 +0300 Andy Shevchenko <[email protected]> wrote:

On Wed, Apr 24, 2024 at 08:51:23PM +0200, Marek Behún wrote:

Quoted 4 lines

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.

Quoted 2 lines

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?

Quoted 3 lines · Marek Behún

Quoted 1 line · On Wed, 24 Apr 2024 22:47:10 +0300 Andy Shevchenko <[email protected]> wrote:

On Wed, Apr 24, 2024 at 08:51:23PM +0200, Marek Behún wrote:

...

Quoted 8 lines

Quoted 4 lines

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.

Quoted 4 lines

Quoted 2 lines

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).

Quoted 3 lines · Marek Behún

Quoted 1 line · On Wed, 24 Apr 2024 21:33:44 +0300 Andy Shevchenko <[email protected]> wrote:

On Wed, Apr 24, 2024 at 07:38:05PM +0200, Marek Behún wrote:

Quoted 9 lines

Quoted 6 lines

drivers/platform/cznic/turris-omnia-mcu-trng.c line 62

Quoted 4 lines

drivers/platform/cznic/turris-omnia-mcu-trng.c new file +4 -0

+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?

Quoted 5 lines

Quoted 4 lines

Quoted 4 lines

drivers/platform/cznic/turris-omnia-mcu-trng.c new file +4 -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");

Quoted 8 lines

Quoted 6 lines

drivers/platform/cznic/turris-omnia-mcu-trng.c line 81

Quoted 4 lines

drivers/platform/cznic/turris-omnia-mcu-trng.c new file +4 -0

+	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.

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.

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 <[email protected]>

---
 .../dts/marvell/armada-385-turris-omnia.dts   | 22 ++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts +21 -1

index 7b755bb4e4e7..59079d63fe27 100644
@@ -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";

Signature · 1 line

2.43.2

Hello Marek,

Quoted 3 lines

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

Quoted 1 line

Fixes 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")

Is it really a fix ?

It seems that the node you add depends on the firmware support that was not yet added.

Gregory

But I am guessing it might be because of what new U-Boot does: if it finds the MCU node in the device-tree, it will add a reset gpio property for ethernet PHY, linking it to the MCU node. So if this patch got backported to older kernel where the MCU driver is not present, and the board had this new U-Boot, the kernel would not create the eth2 interface, because it would be waiting for a driver providing the GPIO.

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 <[email protected]>

---
 .../boot/dts/marvell/armada-385-turris-omnia.dts    | 13 +++++++++++++
 1 file changed, 13 insertions(+)

arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts +13 -0

index 59079d63fe27..43202890c959 100644
@@ -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";

Signature · 1 line

2.43.2

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/[email protected]/

See also cover letters for v1, v2, v3, v4, v5 and v6:
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/

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

Signature · 1 line

2.43.2

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 ?