Skip to contentSkip to conversation

Turris Omnia MCU driver

PATCH Superseded Marek Behún 4 participants 16 messages Oct 23, 2023 – Oct 26, 2023

Subsystems (6)

v3 of 13 Superseded Latest is v13 →

lore
Share & tools
lore

Marek Behún

author maintainer
#0 lore raw

v3 cover Turris Omnia MCU driver

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

this is v3 of series adding support for Turris Omnia MCU. See the cover letter from v1 and v2:
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/

Changes since v2:
- I've dropped the usage of devm_device_add_group(), since it is deprecated as noted by Andy. In patch 4 I use the .is_visible() attribute group method instead
- dropped the setting of min_hw_heartbeat_ms watchdog member in 5/7, since it is not needed, as suggested by Guenter
- addressed Linus' and Andy's suggestions in patches 2/7 and 3/7. There is one of Andy's suggestions which I did not address completely, see changelog directly in patch 3/7

Marek Behún (7):
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
ARM: dts: turris-omnia: Add MCU system-controller node
ARM: dts: turris-omnia: Add GPIO key node for front button

 .../sysfs-bus-i2c-devices-turris-omnia-mcu    |   77 ++
 .../bindings/arm/cznic,turris-omnia-mcu.yaml  |   67 ++
 MAINTAINERS                                   |    4 +
 .../dts/marvell/armada-385-turris-omnia.dts   |   35 +-
 drivers/platform/Kconfig                      |    2 +
 drivers/platform/Makefile                     |    1 +
 drivers/platform/cznic/Kconfig                |   47 +
 drivers/platform/cznic/Makefile               |    7 +
 .../platform/cznic/turris-omnia-mcu-base.c    |  271 +++++
 .../platform/cznic/turris-omnia-mcu-gpio.c    | 1021 +++++++++++++++++
 .../cznic/turris-omnia-mcu-sys-off-wakeup.c   |  250 ++++
 .../cznic/turris-omnia-mcu-watchdog.c         |  122 ++
 drivers/platform/cznic/turris-omnia-mcu.h     |  168 +++
 include/linux/turris-omnia-mcu-interface.h    |  194 ++++
 14 files changed, 2265 insertions(+), 1 deletion(-)
 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-gpio.c
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.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.41.0
author maintainer
#1 lore raw

PATCH v3 1/7 dt-bindings: arm: add cznic,turris-omnia-mcu binding

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.

---
 .../bindings/arm/cznic,turris-omnia-mcu.yaml  | 67 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/cznic,turris-omnia-mcu.yaml
Documentation/devicetree/bindings/arm/cznic,turris-omnia-mcu.yaml new file +67 -0
new file mode 100644
index 000000000000..ea1fd0117bb8
@@ -0,0 +1,67 @@
+# 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 <[email protected]>
+
+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
+
+  gpio-controller: true
+
+  '#gpio-cells':
+    const: 2
+
+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 = <2>;
+
+            interrupt-controller;
+            #interrupt-cells = <2>;
+        };
+    };
MAINTAINERS +1 -0
index 2894f0777537..56cb540bb4ef 100644
@@ -2102,6 +2102,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
Signature · 1 line
2.41.0
author maintainer
#2 lore raw

PATCH v3 2/7 platform: cznic: Add preliminary support for Turris Omnia MCU

In reply to Marek Behún

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

---
 .../sysfs-bus-i2c-devices-turris-omnia-mcu    |  45 ++++
 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    | 253 ++++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |  65 +++++
 include/linux/turris-omnia-mcu-interface.h    | 194 ++++++++++++++
 9 files changed, 593 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
Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu new file +45 -0
new file mode 100644
index 000000000000..72cdf0cfce3e
@@ -0,0 +1,45 @@
+What:		/sys/bus/i2c/devices/<mcu_device>/fw_features
+Date:		November 2023
+KernelVersion:	6.7
+Contact:	Marek Behún <[email protected]>
+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:		November 2023
+KernelVersion:	6.7
+Contact:	Marek Behún <[email protected]>
+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:		November 2023
+KernelVersion:	6.7
+Contact:	Marek Behún <[email protected]>
+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:		November 2023
+KernelVersion:	6.7
+Contact:	Marek Behún <[email protected]>
+Description:	(RO) Contains the microcontroller type (STM32, GD32, MKL).
+
+		Format: %s.
+
+What:		/sys/bus/i2c/devices/<mcu_device>/reset_selector
+Date:		November 2023
+KernelVersion:	6.7
+Contact:	Marek Behún <[email protected]>
+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.
MAINTAINERS +3 -0
index 56cb540bb4ef..8dab94d51c47 100644
@@ -2100,6 +2100,7 @@ M:	Marek Behún <[email protected]>
 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
@@ -2113,10 +2114,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 <[email protected]>
drivers/platform/Kconfig +2 -0
index 868b20361769..fef907a94001 100644
@@ -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"
drivers/platform/Makefile +1 -0
index 41640172975a..8bf189264374 100644
@@ -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/
drivers/platform/cznic/Kconfig new file +26 -0
new file mode 100644
index 000000000000..f8560ff9c1af
@@ -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
drivers/platform/cznic/Makefile new file +4 -0
new file mode 100644
index 000000000000..4d0a9586538c
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+obj-$(CONFIG_TURRIS_OMNIA_MCU)	+= turris-omnia-mcu.o
+turris-omnia-mcu-objs		:= turris-omnia-mcu-base.o
drivers/platform/cznic/turris-omnia-mcu-base.c new file +253 -0

253 lines not on this page · raw message

drivers/platform/cznic/turris-omnia-mcu.h new file +65 -0

65 lines not on this page · raw message

include/linux/turris-omnia-mcu-interface.h new file +194 -0

194 lines not on this page · raw message

Signature · 1 line
2.41.0
author maintainer
#3 lore raw

PATCH v3 3/7 platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs

In reply to 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

---
Changes since v2:
- addressed most of Andy's and Linus' suggestions:
- for interleaving / deinterleaving the bytes in interrupt mask
Andy suggested using his proposed bitmap_gather() and bitmap_scatter()
(from https://lore.kernel.org/linux-gpio/[email protected]/)
Since from the conversation there it seems that there maybe isn't
enough consensus yet, for now I've decided to introduce functions omnia_mask_interleave() and omnia_mask_deinterleave(). These are more
specific for this driver. Once there is consensus for the proposed
bitmap functions, we can change it in this driver
- Andy suggested using devm_delayed_work_autocancel(), but I cannot use
it directly because of a quirk specific for this driver: the order
of IRQ and work request and remove must be:

    INIT_DELAYED_WORK()
    request_threaded_irq()
    ...
    cancel_delayed_work_sync()
    free_irq()
  But using
    devm_delayed_work_autocancel()
    devm_request_threaded_irq()
  would cause improper remove order:
    free_irq()
    cancel_delayed_work_sync()
  I've solved this by doing:
    INIT_DELAYED_WORK()
    devm_request_threaded_irq()
    devm_add_action(..., devm_delayed_work_drop, ...)
  and commented this in the code
---
 .../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    |   13 +-
 .../platform/cznic/turris-omnia-mcu-gpio.c    | 1021 +++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |   61 +-
 6 files changed, 1124 insertions(+), 3 deletions(-)
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-gpio.c
Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu +16 -0
index 72cdf0cfce3e..e635ba45ef9a 100644
@@ -1,3 +1,19 @@
+What:		/sys/bus/i2c/devices/<mcu_device>/front_button_mode
+Date:		November 2023
+KernelVersion:	6.7
+Contact:	Marek Behún <[email protected]>
+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:		November 2023
 KernelVersion:	6.7
drivers/platform/cznic/Kconfig +15 -0
index f8560ff9c1af..3a8c3edcd7e6 100644
@@ -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.
 
drivers/platform/cznic/Makefile +1 -0
index 4d0a9586538c..a6177f5b4fff 100644
@@ -2,3 +2,4 @@
 
 obj-$(CONFIG_TURRIS_OMNIA_MCU)	+= turris-omnia-mcu.o
 turris-omnia-mcu-objs		:= turris-omnia-mcu-base.o
+turris-omnia-mcu-objs		+= turris-omnia-mcu-gpio.o
drivers/platform/cznic/turris-omnia-mcu-base.c +11 -2
index b0eb29b16383..912058694d77 100644
@@ -105,7 +105,16 @@ static struct attribute *omnia_mcu_attrs[] = {
 	&dev_attr_reset_selector.attr,
 	NULL
 };
-ATTRIBUTE_GROUPS(omnia_mcu);
+
+static const struct attribute_group omnia_mcu_base_group = {
+	.attrs = omnia_mcu_attrs,
+};
+
+static const struct attribute_group *omnia_mcu_groups[] = {
+	&omnia_mcu_base_group,
+	&omnia_mcu_gpio_group,
+	NULL
+};
 
 static void omnia_mcu_print_version_hash(struct omnia_mcu *mcu, bool bootloader)
 {
@@ -229,7 +238,7 @@ static int omnia_mcu_probe(struct i2c_client *client)
 		return dev_err_probe(dev, err,
 				     "Cannot determine MCU supported features\n");
 
-	return 0;
+	return omnia_mcu_register_gpiochip(mcu);
 }
 
 static const struct of_device_id of_omnia_mcu_match[] = {
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +1021 -0

1,021 lines not on this page · raw message

drivers/platform/cznic/turris-omnia-mcu.h +60 -1

89 lines not on this page · raw message

Signature · 1 line
2.41.0
#4 lore raw

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

Quoted 9 lines · 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

...

drivers/platform/cznic/turris-omnia-mcu-gpio.c line 442

Quoted 6 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +6 -0
+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;
+	else if (gpio->feat)

Redundant 'else'.

Quoted 4 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +4 -0
+		return mcu->features & gpio->feat;
+
+	return true;
+}

...

Quoted 5 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +5 -0
+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);

drivers/platform/cznic/turris-omnia-mcu-gpio.c line 462

Quoted 10 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +9 -0
+
+	for (unsigned int i = 0; i < ngpios; i++) {
+		const struct omnia_gpio *gpio = &omnia_gpios[i];
 
+		if (!gpio->cmd && !gpio->int_bit) {
+			__clear_bit(i, valid_mask);
+			continue;
+		}
+
+		__assign_bit(i, valid_mask, omnia_gpio_available(mcu, gpio));

Hmm... Why not simply

		if (...)
			__clear_bit();
		else
			__assign_bit(...);

?

Quoted 4 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +4 -0
+	}
+
+	return 0;
+}

...

drivers/platform/cznic/turris-omnia-mcu-gpio.c line 540

Quoted 10 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +10 -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);

I'm wondering if sparse complains on unbalanced locking. If so, the function needs an annotation.

Quoted 1 line

+}

...

Quoted 31 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +31 -0
+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.

drivers/platform/cznic/turris-omnia-mcu-gpio.c line 629

Quoted 10 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +10 -0
+	 * 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);
+}

Same question as above.

...

Quoted 5 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +5 -0
+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);

drivers/platform/cznic/turris-omnia-mcu-gpio.c line 657

Quoted 10 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +10 -0
+
+	for (unsigned int i = 0; i < ngpios; i++) {
+		const struct omnia_gpio *gpio = &omnia_gpios[i];
+
+		if (!gpio->int_bit) {
+			__clear_bit(i, valid_mask);
+			continue;
+		}
+
+		__assign_bit(i, valid_mask, omnia_gpio_available(mcu, gpio));

if-else ?

Quoted 2 lines
(quoted diff) +2 -0
+	}
+}

...

drivers/platform/cznic/turris-omnia-mcu-gpio.c line 924

Quoted 6 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +6 -0
+static void omnia_mcu_mutex_destroy(void *data)
+{
+	struct mutex *lock = data;
+
+	mutex_destroy(lock);
+}

Can be shrunk to oneliner:

static void omnia_mcu_mutex_destroy(void *lock)

{
	mutex_destroy(lock);
}

...

Quoted 13 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +13 -0
+		/*
+		 * 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.
+		 */

...

Quoted 7 lines
drivers/platform/cznic/turris-omnia-mcu.h +7 -0
+/* 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;
+

drivers/platform/cznic/turris-omnia-mcu.h static inline int omnia_cmd_read(const struct i2c_client *client, u8 cmd, void *

Quoted 10 lines
drivers/platform/cznic/turris-omnia-mcu.h +9 -0
+	if (!bits) {
+		*dst = 0;
+		return 0;
+	}
 
+	err = omnia_cmd_read(client, cmd, &reply, (__fls(bits) >> 3) + 1);
+	if (!err)
+		*dst = le32_to_cpu(reply) & bits;
+
+	return err;

Why not be in align with the below, i.e.

	if (err)
		return err;
	...
	return 0;

?

Quoted 14 lines
drivers/platform/cznic/turris-omnia-mcu.h +14 -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;
+}

...

drivers/platform/cznic/turris-omnia-mcu.h static inline int omnia_cmd_read(const struct i2c_client *client, u8 cmd, void * · line 103

Quoted 5 lines

static inline int omnia_cmd_read_u16(const struct i2c_client *client, u8 cmd) {

drivers/platform/cznic/turris-omnia-mcu.h +1 -1
-	u16 reply;
+	__le16 reply;
 	int err;

Shouldn't this be a part of another patch?

Quoted 1 line

}

Signature · 2 lines
With Best Regards,
Andy Shevchenko
author maintainer
#5 lore raw

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

Quoted 1 line · Andy Shevchenko

On Mon, Oct 23, 2023 at 04:31:26PM +0200, Marek Behún wrote:

...

Quoted 5 lines
Quoted 5 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +5 -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);
+
Quoted 8 lines

drivers/platform/cznic/turris-omnia-mcu-gpio.c line 540

Quoted 5 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +5 -0
+	/* nothing to do if MCU firmware does not support new interrupt API */
+	if (!(mcu->features & FEAT_NEW_INT_API))
+		return;
+
+	mutex_lock(&mcu->lock);

I'm wondering if sparse complains on unbalanced locking. If so, the function needs an annotation.

Hi Andy, thanks for your review.

OK so I looked into this and it seems that mutex API does not do __acquire / __release yet...

I've incorporated your other suggestions and am sending v4.

Marek

author maintainer
#6 lore raw

PATCH v3 4/7 platform: cznic: turris-omnia-mcu: Add support for poweroff and wakeup

In reply to 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.

---
 .../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   | 250 ++++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |  20 ++
 6 files changed, 296 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 e635ba45ef9a..a69172d5b998 100644
@@ -14,6 +14,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:		August 2023
+KernelVersion:	6.6
+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:		November 2023
 KernelVersion:	6.7
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 a6177f5b4fff..6f1470d1f673 100644
@@ -3,3 +3,4 @@
 obj-$(CONFIG_TURRIS_OMNIA_MCU)	+= turris-omnia-mcu.o
 turris-omnia-mcu-objs		:= turris-omnia-mcu-base.o
 turris-omnia-mcu-objs		+= turris-omnia-mcu-gpio.o
+turris-omnia-mcu-objs		+= turris-omnia-mcu-sys-off-wakeup.o
drivers/platform/cznic/turris-omnia-mcu-base.c +5 -0
index 912058694d77..942061a0ee66 100644
@@ -113,6 +113,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
 };
 
@@ -238,6 +239,10 @@ static int omnia_mcu_probe(struct i2c_client *client)
 		return dev_err_probe(dev, err,
 				     "Cannot determine MCU supported features\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 +250 -0

250 lines not on this page · raw message

drivers/platform/cznic/turris-omnia-mcu.h +20 -0
index 12afc45ed5bc..c6a8036e0534 100644
@@ -12,9 +12,11 @@
 #include <linux/gpio/driver.h>
 #include <linux/i2c.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;
@@ -29,6 +31,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;
 };
 
 static inline int omnia_cmd_write(const struct i2c_client *client, void *cmd,
@@ -41,6 +48,17 @@ static inline int omnia_cmd_write(const struct i2c_client *client, void *cmd,
 	return ret < 0 ? ret : 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)
 {
@@ -118,7 +136,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.41.0
#7 lore raw

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

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

...

drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c line 86

Quoted 12 lines
drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c new file +11 -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);
 
+	else

Redundant 'else'.

Quoted 2 lines
(quoted diff) +2 -0
+		return 0;
+}

...

Quoted 10 lines
drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c new file +10 -0
+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;

drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c line 125

Quoted 10 lines
drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c new file +9 -0
+	else
+		arg = 0;
+
+	cmd[0] = CMD_POWER_OFF;
+	put_unaligned_le16(CMD_POWER_OFF_MAGIC, &cmd[1]);
+	put_unaligned_le16(arg, &cmd[3]);
 
+	tmp = cpu_to_be32(get_unaligned_le32(&cmd[1]));
+	put_unaligned_le32(crc32_be(0xffffffff, (void *)&tmp, sizeof(tmp)),
+			   &cmd[5]);

Hmm... Why crc32_le*() can't be used?

Quoted 7 lines
drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c new file +7 -0
+	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;
+}

...

drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c line 178

Quoted 2 lines
drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c new file +2 -0
+	if (kstrtobool(buf, &val) < 0)
+		return -EINVAL;

Why is the error code shadowed?

...

drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c line 194

Quoted 1 line
drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c new file +1 -0
+	struct device *dev = container_of(kobj, typeof(*dev), kobj);

kobj_to_dev()?

...

drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c line 223

Quoted 3 lines
drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c new file +3 -0
+	/*
+	 * poweroff and wakeup are available only if POWEROFF_WAKEUP feature is
+	 * present

Missing period at the end.

Quoted 1 line
(quoted diff) +1 -0
+	 */

...

drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c line 249

Quoted 7 lines
drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c new file +6 -0
+	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 err;

Can it be anything than 0?

Signature · 2 lines
With Best Regards,
Andy Shevchenko
author maintainer
#8 lore raw

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

Quoted 1 line · Andy Shevchenko

On Mon, Oct 23, 2023 at 04:31:27PM +0200, Marek Behún wrote:

...

Quoted 5 lines

drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c line 125

Quoted 3 lines
drivers/platform/cznic/turris-omnia-mcu-sys-off-wakeup.c new file +3 -0
+	tmp = cpu_to_be32(get_unaligned_le32(&cmd[1]));
+	put_unaligned_le32(crc32_be(0xffffffff, (void *)&tmp, sizeof(tmp)),
+			   &cmd[5]);

Hmm... Why crc32_le*() can't be used?

I am adding a comment to explain this in v4.
Thx for review.

Marek

author maintainer
#9 lore raw

PATCH v3 5/7 platform: cznic: turris-omnia-mcu: Add support for MCU watchdog

In reply to Marek Behún

Add support for the watchdog mechanism provided by the MCU.

---
 drivers/platform/cznic/Kconfig                |   2 +
 drivers/platform/cznic/Makefile               |   1 +
 .../platform/cznic/turris-omnia-mcu-base.c    |   4 +
 .../cznic/turris-omnia-mcu-watchdog.c         | 122 ++++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |  24 ++++
 5 files changed, 153 insertions(+)
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-watchdog.c
drivers/platform/cznic/Kconfig +2 -0
index 0a752aa654fa..e2649cdecc38 100644
@@ -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
drivers/platform/cznic/Makefile +1 -0
index 6f1470d1f673..a43997a12d74 100644
@@ -4,3 +4,4 @@ obj-$(CONFIG_TURRIS_OMNIA_MCU)	+= turris-omnia-mcu.o
 turris-omnia-mcu-objs		:= turris-omnia-mcu-base.o
 turris-omnia-mcu-objs		+= turris-omnia-mcu-gpio.o
 turris-omnia-mcu-objs		+= turris-omnia-mcu-sys-off-wakeup.o
+turris-omnia-mcu-objs		+= turris-omnia-mcu-watchdog.o
drivers/platform/cznic/turris-omnia-mcu-base.c +4 -0
index 942061a0ee66..521397bfc022 100644
@@ -243,6 +243,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);
 }
 
drivers/platform/cznic/turris-omnia-mcu-watchdog.c new file +122 -0

122 lines not on this page · raw message

drivers/platform/cznic/turris-omnia-mcu.h +24 -0
index c6a8036e0534..db2d18d6eea8 100644
@@ -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>
@@ -36,6 +37,9 @@ struct omnia_mcu {
 	struct rtc_device *rtcdev;
 	u32 rtc_alarm;
 	bool front_button_poweron;
+
+	/* MCU watchdog */
+	struct watchdog_device wdt;
 };
 
 static inline int omnia_cmd_write(const struct i2c_client *client, void *cmd,
@@ -48,6 +52,25 @@ static inline int omnia_cmd_write(const struct i2c_client *client, void *cmd,
 	return ret < 0 ? ret : 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)
 {
@@ -140,5 +163,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 */
Signature · 1 line
2.41.0
maintainer
#10 lore raw

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

Quoted 3 lines · Marek Behún

Add support for the watchdog mechanism provided by the MCU.

Any reason for keeping this out of drivers/watchdog ?

Quoted 141 lines
---
  drivers/platform/cznic/Kconfig                |   2 +
  drivers/platform/cznic/Makefile               |   1 +
  .../platform/cznic/turris-omnia-mcu-base.c    |   4 +
  .../cznic/turris-omnia-mcu-watchdog.c         | 122 ++++++++++++++++++
  drivers/platform/cznic/turris-omnia-mcu.h     |  24 ++++
  5 files changed, 153 insertions(+)

create mode 100644 drivers/platform/cznic/turris-omnia-mcu-watchdog.c

drivers/platform/cznic/Kconfig +2 -0
index 0a752aa654fa..e2649cdecc38 100644
@@ -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
drivers/platform/cznic/Makefile +1 -0
index 6f1470d1f673..a43997a12d74 100644
@@ -4,3 +4,4 @@ obj-$(CONFIG_TURRIS_OMNIA_MCU)	+= turris-omnia-mcu.o
  turris-omnia-mcu-objs		:= turris-omnia-mcu-base.o
  turris-omnia-mcu-objs		+= turris-omnia-mcu-gpio.o
  turris-omnia-mcu-objs		+= turris-omnia-mcu-sys-off-wakeup.o
+turris-omnia-mcu-objs		+= turris-omnia-mcu-watchdog.o
drivers/platform/cznic/turris-omnia-mcu-base.c +4 -0
index 942061a0ee66..521397bfc022 100644
@@ -243,6 +243,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);
  }
 
drivers/platform/cznic/turris-omnia-mcu-watchdog.c new file +82 -0
new file mode 100644
index 000000000000..6685166bc4c7
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CZ.NIC's Turris Omnia MCU watchdog driver
+ *
+ * 2023 by Marek Behún <[email protected]>
+ */
+
+#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,

drivers/platform/cznic/turris-omnia-mcu-watchdog.c line 92

Quoted 10 lines
drivers/platform/cznic/turris-omnia-mcu-watchdog.c new file +10 -0
+};
+
+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;
+

Why check this here and not in the calling code ?

Quoted 43 lines
drivers/platform/cznic/turris-omnia-mcu-watchdog.c new file +30 -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;
+}
drivers/platform/cznic/turris-omnia-mcu.h +1 -0
index c6a8036e0534..db2d18d6eea8 100644
@@ -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>

drivers/platform/cznic/turris-omnia-mcu.h struct omnia_mcu { · line 42

Quoted 6 lines
drivers/platform/cznic/turris-omnia-mcu.h +3 -0
@@ -36,6 +37,9 @@ struct omnia_mcu {
  	struct rtc_device *rtcdev;
  	u32 rtc_alarm;
  	bool front_button_poweron;
+
+	/* MCU watchdog */
+	struct watchdog_device wdt;

This should be internal to the watchdog driver.

Quoted 36 lines
drivers/platform/cznic/turris-omnia-mcu.h +20 -0
  };
 
  static inline int omnia_cmd_write(const struct i2c_client *client, void *cmd,
@@ -48,6 +52,25 @@ static inline int omnia_cmd_write(const struct i2c_client *client, void *cmd,
  	return ret < 0 ? ret : 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)
  {
@@ -140,5 +163,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 */
author maintainer
#11 lore raw

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

Quoted 6 lines · On Mon, 23 Oct 2023 07:47:44 -0700 Guenter Roeck <[email protected]> wrote:
Quoted 3 lines · Marek Behún

Add support for the watchdog mechanism provided by the MCU.

Any reason for keeping this out of drivers/watchdog ?

Because this whole thing is compiled as one driver, turris-omnia-mcu.ko. The whole driver is specific for one device, the Turris Omnia router. Splitting it into separate modules would be unnecessary, the driver is meant to provide all the Turris Omnia MCU functionality.

I've seen this done in various platform drivers.

Moreover, all of the functionality lives at one I2C address, so only one I2C client is used.

Originally it was one .c file, but I split it into 4 files to make it easier to review.

You can look at the other patches at
https://patchwork.kernel.org/project/linux-soc/list/?series=795666

Quoted 5 lines

drivers/platform/cznic/turris-omnia-mcu-watchdog.c line 92

Quoted 3 lines
drivers/platform/cznic/turris-omnia-mcu-watchdog.c new file +3 -0
+	if (!(mcu->features & FEAT_WDT_PING))
+		return 0;
+  

Why check this here and not in the calling code ?

No particular reason, just wanted to keep the calling code cleaner.

Quoted 9 lines

drivers/platform/cznic/turris-omnia-mcu.h struct omnia_mcu { · line 42

Quoted 7 lines
drivers/platform/cznic/turris-omnia-mcu.h +3 -0
@@ -36,6 +37,9 @@ struct omnia_mcu {
  	struct rtc_device *rtcdev;
  	u32 rtc_alarm;
  	bool front_button_poweron;
+
+	/* MCU watchdog */
+	struct watchdog_device wdt;  

This should be internal to the watchdog driver.

As explained above, all functionality is provided in one kernel module. This makes sense here since the driver is meant to provide all MCU functionality and it is specific for Turris Omnia.

Marek

#12 lore raw

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

Quoted 12 lines · Marek Behún
Quoted 5 lines · Guenter Roeck
Quoted 1 line · Marek Behún

Add support for the watchdog mechanism provided by the MCU.

Any reason for keeping this out of drivers/watchdog ?

Because this whole thing is compiled as one driver, turris-omnia-mcu.ko. The whole driver is specific for one device, the Turris Omnia router. Splitting it into separate modules would be unnecessary, the driver is meant to provide all the Turris Omnia MCU functionality.

Quoted 4 lines

I've seen this done in various platform drivers.

Moreover, all of the functionality lives at one I2C address, so only one I2C client is used.

That all sounds a bit like drivers/mfd...

author maintainer
#13 lore raw

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

Quoted 13 lines · On Mon, 23 Oct 2023 16:03:08 +0100 Mark Brown <[email protected]> wrote:
Quoted 11 lines · Marek Behún
Quoted 5 lines · Guenter Roeck
Quoted 1 line · Marek Behún

Add support for the watchdog mechanism provided by the MCU.

Any reason for keeping this out of drivers/watchdog ?

Because this whole thing is compiled as one driver, turris-omnia-mcu.ko. The whole driver is specific for one device, the Turris Omnia router. Splitting it into separate modules would be unnecessary, the driver is meant to provide all the Turris Omnia MCU functionality.

Quoted 6 lines
Quoted 4 lines

I've seen this done in various platform drivers.

Moreover, all of the functionality lives at one I2C address, so only one I2C client is used.

That all sounds a bit like drivers/mfd...

Hi Guenter,

I would have thought so as well, but several years ago I sent a driver into mfd, and I've been told (I don't remember by who anymore, but I can find it) that mfd is supposed to contain drivers for multifunction devices that can be contained on different boards, and that my driver should go to platform, since it is specific for one hardware.

Marek

author maintainer
#14 lore raw

PATCH v3 6/7 ARM: dts: turris-omnia: Add MCU system-controller node

In reply to Marek Behún

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")
---
 .../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 2d8d319bec83..7682fda4000a 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 = <2>;
+
+				interrupt-controller;
+				#interrupt-cells = <2>;
+			};
 
 			led-controller@2b {
 				compatible = "cznic,turris-omnia-leds";
@@ -503,6 +518,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.41.0
author maintainer
#15 lore raw

PATCH v3 7/7 ARM: dts: turris-omnia: Add GPIO key node for front button

In reply to Marek Behún

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

Fixes 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")
---
 .../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 7682fda4000a..016adfa0b470 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 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.41.0