Skip to contentSkip to conversation

Turris Omnia MCU driver

PATCH Superseded Marek Behún 3 participants 22 messages May 8, 2024 – May 10, 2024

Subsystems (7)

v9 of 13 Superseded Latest is v13 →

lore
Share & tools
lore

Marek Behún

author maintainer
#0 lore raw

v9 cover Turris Omnia MCU driver

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

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

This series still 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, v6, v7 and v8:
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]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/

Changes since v8:
- removed the MACH_ARMADA_38X || COMPILE_TEST dependency for the CZNIC_PLATFORMS Kconfig option in the Kconfig file, as suggested by Andy (patch 2)
- fixed the issues in the global header turris-omnia-mcu-interface.h pointed out by Andy and Ilpo: added the "OMNIA_" prefix to all enumerator entries, added trailing commas, converted to use FIELD_PREP_CONST() (patches 2-7)
- added comment why we can't use ether_addr_copy(), as requested by Andy (patch 2)
- rewritten the error message for when the MCU does not support reporting features, requested by Andy (patch 2)
- changed how the nul-byte is assigned to the end of bin2hex() result, requested by Andy (patch 2)
- made the omnia_cmd_write_read() function final in patch 2, instead of updating it in subsequent patches, requested by Andy
- in order to be more consistent, changed the omnia_cmd_read_u8() and omnia_cmd_read_u16() functions to return zero on success and the read value is passed into a pointer, and then added a new function omnia_cmd_read_u32(), as requested by Ilpo (he did not like the inconsistency in omnia_mcu_read_features(), where the omnia_cmd_read_u16() hid the le16_to_cpu() call, but then we did an explicit call to le32_to_cpu() when reading 32-bit features) (patch 2)
- some other minor tweaks (changed the version type from u8[] to char[], changed len type from size_t to unsigned int in omnia_cmd_write()) (patches 2 & 3)
- dropped the "gpio%u." prefix from GPIO line names, Andy pointed out why this is wrong (patch 3)
- decoupled GPIOs information struct definition and assignment and put filling macros before the assignment, requested by Andy (patch 3)
- moved return value check into the guarded scope, suggested by Andy (patch 3)
- put some assignments into the definition lines, suggested by Andy (patch 3)
- fixed some typos (patches 2 & 3)
- refactored the reply length computation into a new function omnia_compute_reply_length(), as requested by Andy (patch 3)

Marek Behún (9):
dt-bindings: firmware: add cznic,turris-omnia-mcu binding
platform: cznic: Add preliminary support for Turris Omnia MCU
platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs
platform: cznic: turris-omnia-mcu: Add support for poweroff and wakeup
platform: cznic: turris-omnia-mcu: Add support for MCU watchdog
platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG
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                |   50 +
 drivers/platform/cznic/Makefile               |    9 +
 .../platform/cznic/turris-omnia-mcu-base.c    |  451 +++++++
 .../platform/cznic/turris-omnia-mcu-debugfs.c |  207 ++++
 .../platform/cznic/turris-omnia-mcu-gpio.c    | 1040 +++++++++++++++++
 .../cznic/turris-omnia-mcu-sys-off-wakeup.c   |  258 ++++
 .../platform/cznic/turris-omnia-mcu-trng.c    |  103 ++
 .../cznic/turris-omnia-mcu-watchdog.c         |  127 ++
 drivers/platform/cznic/turris-omnia-mcu.h     |  210 ++++
 include/linux/turris-omnia-mcu-interface.h    |  249 ++++
 17 files changed, 2971 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
author maintainer
#1 lore raw

PATCH v9 1/9 dt-bindings: firmware: 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.

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

PATCH v9 2/9 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    |  81 ++++
 MAINTAINERS                                   |   3 +
 drivers/platform/Kconfig                      |   2 +
 drivers/platform/Makefile                     |   1 +
 drivers/platform/cznic/Kconfig                |  25 ++
 drivers/platform/cznic/Makefile               |   4 +
 .../platform/cznic/turris-omnia-mcu-base.c    | 394 ++++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |  70 ++++
 include/linux/turris-omnia-mcu-interface.h    | 249 +++++++++++
 9 files changed, 829 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 +81 -0
new file mode 100644
index 000000000000..8245c1f15c71
@@ -0,0 +1,81 @@
+What:		/sys/bus/i2c/devices/<mcu_device>/board_revision
+Date:		July 2024
+KernelVersion:	6.10
+Contact:	Marek Behún <[email protected]>
+Description:	(RO) Contains board revision number.
+
+		Only available if board information is burned in the MCU (older
+		revisions have board information burned in the ATSHA204-A chip).
+
+		Format: %u.
+
+What:		/sys/bus/i2c/devices/<mcu_device>/first_mac_address
+Date:		July 2024
+KernelVersion:	6.10
+Contact:	Marek Behún <[email protected]>
+Description:	(RO) Contains device first MAC address. Each Turris Omnia is
+		allocated 3 MAC addresses. The two additional addresses are
+		computed from the first one by incrementing it.
+
+		Only available if board information is burned in the MCU (older
+		revisions have board information burned in the ATSHA204-A chip).
+
+		Format: %pM.
+
+What:		/sys/bus/i2c/devices/<mcu_device>/fw_features
+Date:		July 2024
+KernelVersion:	6.10
+Contact:	Marek Behún <[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:		July 2024
+KernelVersion:	6.10
+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:		July 2024
+KernelVersion:	6.10
+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:		July 2024
+KernelVersion:	6.10
+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:		July 2024
+KernelVersion:	6.10
+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.
+
+What:		/sys/bus/i2c/devices/<mcu_device>/serial_number
+Date:		July 2024
+KernelVersion:	6.10
+Contact:	Marek Behún <[email protected]>
+Description:	(RO) Contains the 64-bit board serial number in hexadecimal
+		format.
+
+		Only available if board information is burned in the MCU (older
+		revisions have board information burned in the ATSHA204-A chip).
+
+		Format: %016X.
MAINTAINERS +3 -0
index aba40663f3f8..b27cc5feb64f 100644
@@ -2139,6 +2139,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/bus/moxtet.txt
@@ -2152,10 +2153,12 @@ F:	drivers/firmware/turris-mox-rwtm.c
 F:	drivers/gpio/gpio-moxtet.c
 F:	drivers/leds/leds-turris-omnia.c
 F:	drivers/mailbox/armada-37xx-rwtm-mailbox.c
+F:	drivers/platform/cznic/
 F:	drivers/watchdog/armada_37xx_wdt.c
 F:	include/dt-bindings/bus/moxtet.h
 F:	include/linux/armada-37xx-rwtm-mailbox.h
 F:	include/linux/moxtet.h
+F:	include/linux/turris-omnia-mcu-interface.h
 
 ARM/FARADAY FA526 PORT
 M:	Hans Ulli Kroll <[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 +25 -0
new file mode 100644
index 000000000000..db5f4a673d28
@@ -0,0 +1,25 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# For a description of the syntax of this configuration file,
+# see Documentation/kbuild/kconfig-language.rst.
+#
+
+menuconfig CZNIC_PLATFORMS
+	bool "Platform support for CZ.NIC's Turris hardware"
+	help
+	  Say Y here to be able to choose driver support for CZ.NIC's Turris
+	  devices. This option alone does not add any kernel code.
+
+if CZNIC_PLATFORMS
+
+config TURRIS_OMNIA_MCU
+	tristate "Turris Omnia MCU driver"
+	depends on MACH_ARMADA_38X || COMPILE_TEST
+	depends on I2C
+	help
+	  Say Y here to add support for the features implemented by the
+	  microcontroller on the CZ.NIC's Turris Omnia SOHO router.
+	  To compile this driver as a module, choose M here; the module will be
+	  called turris-omnia-mcu.
+
+endif # CZNIC_PLATFORMS
drivers/platform/cznic/Makefile new file +4 -0
new file mode 100644
index 000000000000..31adca73bb94
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+obj-$(CONFIG_TURRIS_OMNIA_MCU)	+= turris-omnia-mcu.o
+turris-omnia-mcu-y		:= turris-omnia-mcu-base.o
drivers/platform/cznic/turris-omnia-mcu-base.c new file +394 -0

394 lines not on this page · raw message

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

70 lines not on this page · raw message

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

249 lines not on this page · raw message

Signature · 1 line
2.43.2
#3 lore raw

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

Quoted 2 lines · Marek Behún

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

Some cosmetics in case you need a new version.
Possibly can be done as follow up(s).

...

Review

Quoted 2 lines
(quoted diff) +2 -0
+Date:		July 2024
+KernelVersion:	6.10

TBH, I'm not sure you manage to squeeze this rather big driver to v6.10.

...

drivers/platform/cznic/turris-omnia-mcu-base.c line 200

Quoted 4 lines
drivers/platform/cznic/turris-omnia-mcu-base.c new file +4 -0
+static const struct attribute_group *omnia_mcu_groups[] = {
+	&omnia_mcu_base_group,
+	NULL
+};

__ATTRIBUTE_GROUPS()

...

Perhaps also

	struct i2c_client *client = mcu->client;

Review

Quoted 1 line
(quoted diff) +1 -0
+	struct device *dev = &mcu->client->dev;
	struct device *dev = &client->dev;

drivers/platform/cznic/turris-omnia-mcu-base.c line 262

Quoted 7 lines
drivers/platform/cznic/turris-omnia-mcu-base.c new file +7 -0
+	bool suggest_fw_upgrade = false;
+	u16 status;
+	int err;
+
+	/* status word holds MCU type, which we need below */
+	err = omnia_cmd_read_u16(mcu->client, OMNIA_CMD_GET_STATUS_WORD,
+				 &status);
	err = omnia_cmd_read_u16(client, OMNIA_CMD_GET_STATUS_WORD, &status);

and so on...

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

...

drivers/platform/cznic/turris-omnia-mcu-base.c line 267

Quoted 2 lines
drivers/platform/cznic/turris-omnia-mcu-base.c new file +2 -0
+	/*
+	 * check whether MCU firmware supports the OMNIA_CMD_GET_FEATURES

Check

drivers/platform/cznic/turris-omnia-mcu-base.c line 268

Quoted 1 line
drivers/platform/cznic/turris-omnia-mcu-base.c new file +1 -0
+	 * command

command.

Quoted 1 line
(quoted diff) +1 -0
+	 */
Signature · 2 lines
With Best Regards,
Andy Shevchenko
author maintainer
#4 lore raw

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

Quoted 14 lines · Andy Shevchenko
Quoted 2 lines · Marek Behún

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

Some cosmetics in case you need a new version.
Possibly can be done as follow up(s).

...

Review

Quoted 2 lines
(quoted diff) +2 -0
+Date:		July 2024
+KernelVersion:	6.10

TBH, I'm not sure you manage to squeeze this rather big driver to v6.10.

Quoted 8 lines

...

drivers/platform/cznic/turris-omnia-mcu-base.c line 200

Quoted 4 lines
drivers/platform/cznic/turris-omnia-mcu-base.c new file +4 -0
+static const struct attribute_group *omnia_mcu_groups[] = {
+	&omnia_mcu_base_group,
+	NULL
+};

__ATTRIBUTE_GROUPS()

I think we already discussed this one. __ATTRIBUTE_GROUPS defines only one group, and I am adding to the array in subsequent patches.

Quoted 46 lines

...

Perhaps also

	struct i2c_client *client = mcu->client;

Review

Quoted 1 line
(quoted diff) +1 -0
+	struct device *dev = &mcu->client->dev;
	struct device *dev = &client->dev;

drivers/platform/cznic/turris-omnia-mcu-base.c line 262

Quoted 7 lines
drivers/platform/cznic/turris-omnia-mcu-base.c new file +7 -0
+	bool suggest_fw_upgrade = false;
+	u16 status;
+	int err;
+
+	/* status word holds MCU type, which we need below */
+	err = omnia_cmd_read_u16(mcu->client, OMNIA_CMD_GET_STATUS_WORD,
+				 &status);
	err = omnia_cmd_read_u16(client, OMNIA_CMD_GET_STATUS_WORD, &status);

and so on...

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

...

drivers/platform/cznic/turris-omnia-mcu-base.c line 267

Quoted 2 lines
drivers/platform/cznic/turris-omnia-mcu-base.c new file +2 -0
+	/*
+	 * check whether MCU firmware supports the OMNIA_CMD_GET_FEATURES

Check

drivers/platform/cznic/turris-omnia-mcu-base.c line 268

Quoted 1 line
drivers/platform/cznic/turris-omnia-mcu-base.c new file +1 -0
+	 * command

command.

Quoted 1 line
(quoted diff) +1 -0
+	 */
Signature · 2 lines
With Best Regards,
Andy Shevchenko
author maintainer
#5 lore raw

PATCH v9 3/9 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

---
 .../sysfs-bus-i2c-devices-turris-omnia-mcu    |   16 +
 drivers/platform/cznic/Kconfig                |   15 +
 drivers/platform/cznic/Makefile               |    1 +
 .../platform/cznic/turris-omnia-mcu-base.c    |    3 +-
 .../platform/cznic/turris-omnia-mcu-gpio.c    | 1037 +++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |   65 ++
 6 files changed, 1136 insertions(+), 1 deletion(-)
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-gpio.c
Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu +16 -0
index 8245c1f15c71..3fb998c8ae26 100644
@@ -22,6 +22,22 @@ Description:	(RO) Contains device first MAC address. Each Turris Omnia is
 
 		Format: %pM.
 
+What:		/sys/bus/i2c/devices/<mcu_device>/front_button_mode
+Date:		July 2024
+KernelVersion:	6.10
+Contact:	Marek Behún <[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:		July 2024
 KernelVersion:	6.10
drivers/platform/cznic/Kconfig +15 -0
index db5f4a673d28..d95e7c83c7ae 100644
@@ -16,9 +16,24 @@ config TURRIS_OMNIA_MCU
 	tristate "Turris Omnia MCU driver"
 	depends on MACH_ARMADA_38X || COMPILE_TEST
 	depends on I2C
+	select GPIOLIB
+	select GPIOLIB_IRQCHIP
 	help
 	  Say Y here to add support for the features implemented by the
 	  microcontroller on the CZ.NIC's Turris Omnia SOHO router.
+	  The features include:
+	  - GPIO pins
+	    - to get front button press events (the front button can be
+	      configured either to generate press events to the CPU or to change
+	      front LEDs panel brightness)
+	    - to enable / disable USB port voltage regulators and to detect
+	      USB overcurrent
+	    - to detect MiniPCIe / mSATA card presence in MiniPCIe port 0
+	    - to configure resets of various peripherals on board revisions 32+
+	    - to enable / disable the VHV voltage regulator to the SOC in order
+	      to be able to program SOC's OTP on board revisions 32+
+	    - to get input from the LED output pins of the WAN ethernet PHY, LAN
+	      switch and MiniPCIe ports
 	  To compile this driver as a module, choose M here; the module will be
 	  called turris-omnia-mcu.
 
drivers/platform/cznic/Makefile +1 -0
index 31adca73bb94..53fd8f1777a3 100644
@@ -2,3 +2,4 @@
 
 obj-$(CONFIG_TURRIS_OMNIA_MCU)	+= turris-omnia-mcu.o
 turris-omnia-mcu-y		:= turris-omnia-mcu-base.o
+turris-omnia-mcu-y		+= turris-omnia-mcu-gpio.o
drivers/platform/cznic/turris-omnia-mcu-base.c +2 -1
index 9b5acb857e07..45971228e343 100644
@@ -196,6 +196,7 @@ static const struct attribute_group omnia_mcu_base_group = {
 
 static const struct attribute_group *omnia_mcu_groups[] = {
 	&omnia_mcu_base_group,
+	&omnia_mcu_gpio_group,
 	NULL
 };
 
@@ -371,7 +372,7 @@ static int omnia_mcu_probe(struct i2c_client *client)
 					     "Cannot read board info\n");
 	}
 
-	return 0;
+	return omnia_mcu_register_gpiochip(mcu);
 }
 
 static const struct of_device_id of_omnia_mcu_match[] = {
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +1037 -0

1,037 lines not on this page · raw message

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

95 lines not on this page · raw message

Signature · 1 line
2.43.2
#6 lore raw

Re: [PATCH v9 3/9] 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-base.c static const struct attribute_group omnia_mcu_base_group = { · line 201

Quoted 5 lines

static const struct attribute_group *omnia_mcu_groups[] = {

drivers/platform/cznic/turris-omnia-mcu-base.c +1 -0
 	&omnia_mcu_base_group,
+	&omnia_mcu_gpio_group,
 	NULL
 };

Ah, __ATTRIBUTE_GROUPS() can't be used.

...

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

Quoted 2 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +2 -0
+static int omnia_ctl_cmd_locked(struct omnia_mcu *mcu, u8 cmd, u16 val,
+				u16 mask)

Can be one line as it's only 81 characters long.

...

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

Quoted 9 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +9 -0
+	if (type & IRQ_TYPE_EDGE_RISING)
+		mcu->rising |= bit;
+	else
+		mcu->rising &= ~bit;
+
+	if (type & IRQ_TYPE_EDGE_FALLING)
+		mcu->falling |= bit;
+	else
+		mcu->falling &= ~bit;

If those variables was defined as unsigned long, these can be just

	__assign_bit()
	__assign_bit()

And other non-atomic bitops elsewhere, like __clear_bit().

...

Quoted 2 lines

+ * Feel free to remove this function and its inverse, omnia_mask_deinterleave,
+ * and use an appropriate bitmap_* function once such a function exists.

bitmap_*()

...

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

Quoted 6 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +6 -0
+static void omnia_mask_interleave(u8 *dst, u32 rising, u32 falling)
+{
+	for (int i = 0; i < sizeof(u32); ++i) {
+		dst[2 * i] = rising >> (8 * i);
+		dst[2 * i + 1] = falling >> (8 * i);
+	}

Yeah, this can actually be converted to the existing bitmap ops, but I think it will be a bit an overkill for now. Perhaps we will have something better in the future.

Quoted 1 line

+}

+ blank line. (Or is it me who accidentally removed it?)

Quoted 17 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +17 -0
+/**
+ * omnia_mask_deinterleave - Deinterleaves the bytes into @rising and @falling
+ *	@src: the source u8 array containing the interleaved bytes
+ *	@rising: pointer where to store the rising mask gathered from @src
+ *	@falling: pointer where to store the falling mask gathered from @src
+ *
+ * This is the inverse function to omnia_mask_interleave.
+ */
+static void omnia_mask_deinterleave(const u8 *src, u32 *rising, u32 *falling)
+{
+	*rising = *falling = 0;
+
+	for (int i = 0; i < sizeof(u32); ++i) {
+		*rising |= src[2 * i] << (8 * i);
+		*falling |= src[2 * i + 1] << (8 * i);
+	}
+}

...

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

Quoted 7 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +7 -0
+static int omnia_read_status_word_old_fw(struct omnia_mcu *mcu, u16 *status)
+{
+	int err;
+
+	err = omnia_cmd_read_u16(mcu->client, OMNIA_CMD_GET_STATUS_WORD,
+				 status);
+	if (!err)

Why not traditional pattern?

	if (err)
		return err;

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

Quoted 7 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +7 -0
+		/*
+		 * Old firmware has a bug wherein it never resets the USB port
+		 * overcurrent bits back to zero. Ignore them.
+		 */
+		*status &= ~(OMNIA_STS_USB30_OVC | OMNIA_STS_USB31_OVC);
+
+	return err;
	return 0;
Quoted 1 line

+}

...

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

Quoted 6 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +6 -0
+static bool omnia_irq_read_pending(struct omnia_mcu *mcu,
+				   unsigned long *pending)
+{
+	if (mcu->features & OMNIA_FEAT_NEW_INT_API)
+		return omnia_irq_read_pending_new(mcu, pending);
+	else

'else' is redundant, but it can be still used for indentation purposes here.

Quoted 2 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +2 -0
+		return omnia_irq_read_pending_old(mcu, pending);
+}

...

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

Quoted 8 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +8 -0
+static struct attribute *omnia_mcu_gpio_attrs[] = {
+	&dev_attr_front_button_mode.attr,
+	NULL
+};
+
+const struct attribute_group omnia_mcu_gpio_group = {
+	.attrs = omnia_mcu_gpio_attrs,
+};

Haven't seen the rest, but here perhaps ATTRIBUTE_GROUPS().

...

Quoted 5 lines
drivers/platform/cznic/turris-omnia-mcu.h +5 -0
+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 struct omnia_mcu { · line 80

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,
+			     omnia_compute_reply_length(bits, false, 0));
 
+	if (!err)
	if (err)
		return err;

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

Quoted 3 lines
drivers/platform/cznic/turris-omnia-mcu.h +3 -0
+		*dst = le32_to_cpu(reply) & bits;
+
+	return err;
	return 0;
Quoted 1 line

+}

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

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

Quoted 1 line · On Wed, 8 May 2024 14:16:26 +0300 Andy Shevchenko <[email protected]> wrote:

On Wed, May 08, 2024 at 12:31:12PM +0200, Marek Behún wrote:

...

Quoted 4 lines

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

Quoted 2 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +2 -0
+static int omnia_ctl_cmd_locked(struct omnia_mcu *mcu, u8 cmd, u16 val,
+				u16 mask)  

Can be one line as it's only 81 characters long.

OK

Quoted 12 lines

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

Quoted 9 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +9 -0
+	if (type & IRQ_TYPE_EDGE_RISING)
+		mcu->rising |= bit;
+	else
+		mcu->rising &= ~bit;
+
+	if (type & IRQ_TYPE_EDGE_FALLING)
+		mcu->falling |= bit;
+	else
+		mcu->falling &= ~bit;  

If those variables was defined as unsigned long, these can be just

Quoted 4 lines
	__assign_bit()
	__assign_bit()

And other non-atomic bitops elsewhere, like __clear_bit().

Changing this propagated to many other variables and even required some refactoring of the omnia_gpio structure, since the bit, ctl_bit and int_bit members are stored as a masks, but __assign_bit() / __set_bit() / __clear_bit() requires bit numbers.

For example
if (gpio->int_bit && (mcu->is_cached & gpio->int_bit))

    return !!(mcu->cached & gpio->int_bit);
needed to change to
  if (gpio->has_int && (mcu->is_cached & BIT(gpio->int_bit)))
    return !!(mcu->cached & BIT(gpio->int_bit));

and so on.

Moreover, I agree that the if-else statement which you commented on, when changed to __assign_bit(), looks much nicer, but some changes that sprouted from this are in my opinion less readable.

I have prepared the fixup patch, but I am not confident enough that everything is done correctly. I would prefer leaving this one for later, if it is okay with you.

Quoted 4 lines
Quoted 2 lines

+ * Feel free to remove this function and its inverse, omnia_mask_deinterleave,
+ * and use an appropriate bitmap_* function once such a function exists.

bitmap_*()

OK

...

Quoted 10 lines

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

Quoted 7 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +7 -0
+static int omnia_read_status_word_old_fw(struct omnia_mcu *mcu, u16 *status)
+{
+	int err;
+
+	err = omnia_cmd_read_u16(mcu->client, OMNIA_CMD_GET_STATUS_WORD,
+				 status);
+	if (!err)  

Why not traditional pattern?

Quoted 2 lines
	if (err)
		return err;

OK, also for the rest similar.

...

Quoted 8 lines

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

Quoted 6 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +6 -0
+static bool omnia_irq_read_pending(struct omnia_mcu *mcu,
+				   unsigned long *pending)
+{
+	if (mcu->features & OMNIA_FEAT_NEW_INT_API)
+		return omnia_irq_read_pending_new(mcu, pending);
+	else  

'else' is redundant, but it can be still used for indentation purposes here.

As you say, for indentation purposes I would prefer keeping it this way.

Quoted 6 lines
Quoted 3 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +2 -0
+		return omnia_irq_read_pending_old(mcu, pending);
+}  

...

Quoted 10 lines

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

Quoted 8 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +8 -0
+static struct attribute *omnia_mcu_gpio_attrs[] = {
+	&dev_attr_front_button_mode.attr,
+	NULL
+};
+
+const struct attribute_group omnia_mcu_gpio_group = {
+	.attrs = omnia_mcu_gpio_attrs,
+};  

Haven't seen the rest, but here perhaps ATTRIBUTE_GROUPS().

Those define the variable as static, but I need to access it from turris-omnia-mcu-base.c compilation unit.

#8 lore raw

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

Quoted 3 lines · Marek Behún
Quoted 1 line · On Wed, 8 May 2024 14:16:26 +0300 Andy Shevchenko <[email protected]> wrote:

On Wed, May 08, 2024 at 12:31:12PM +0200, Marek Behún wrote:

...

Quoted 22 lines
Quoted 10 lines

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

Quoted 9 lines
drivers/platform/cznic/turris-omnia-mcu-gpio.c new file +9 -0
+   if (type & IRQ_TYPE_EDGE_RISING)
+           mcu->rising |= bit;
+   else
+           mcu->rising &= ~bit;
+
+   if (type & IRQ_TYPE_EDGE_FALLING)
+           mcu->falling |= bit;
+   else
+           mcu->falling &= ~bit;
Quoted 6 lines

If those variables was defined as unsigned long, these can be just

      __assign_bit()
      __assign_bit()

And other non-atomic bitops elsewhere, like __clear_bit().

Changing this propagated to many other variables and even required some refactoring of the omnia_gpio structure, since the bit, ctl_bit and int_bit members are stored as a masks, but __assign_bit() / __set_bit() / __clear_bit() requires bit numbers.

Quoted 6 lines

For example
if (gpio->int_bit && (mcu->is_cached & gpio->int_bit))

    return !!(mcu->cached & gpio->int_bit);
needed to change to
  if (gpio->has_int && (mcu->is_cached & BIT(gpio->int_bit)))
    return !!(mcu->cached & BIT(gpio->int_bit));

You rather meant

  return test_bit(...);
Quoted 1 line

and so on.

...and so on :-)

Quoted 3 lines

Moreover, I agree that the if-else statement which you commented on, when changed to __assign_bit(), looks much nicer, but some changes that sprouted from this are in my opinion less readable.

Maybe. It can be done later on.

Quoted 3 lines

I have prepared the fixup patch, but I am not confident enough that everything is done correctly. I would prefer leaving this one for later, if it is okay with you.

--
With Best Regards,
Andy Shevchenko

author maintainer
#9 lore raw

PATCH v9 4/9 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   | 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 3fb998c8ae26..5e2d8ec52374 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 d95e7c83c7ae..c1e719235517 100644
@@ -18,10 +18,14 @@ config TURRIS_OMNIA_MCU
 	depends on I2C
 	select GPIOLIB
 	select GPIOLIB_IRQCHIP
+	select RTC_CLASS
 	help
 	  Say Y here to add support for the features implemented by the
 	  microcontroller on the CZ.NIC's Turris Omnia SOHO router.
 	  The features include:
+	  - board poweroff into true low power mode (with voltage regulators
+	    disabled) and the ability to configure wake up from this mode (via
+	    rtcwake)
 	  - GPIO pins
 	    - to get front button press events (the front button can be
 	      configured either to generate press events to the CPU or to change
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 45971228e343..32d187e12261 100644
@@ -197,6 +197,7 @@ static const struct attribute_group omnia_mcu_base_group = {
 static const struct attribute_group *omnia_mcu_groups[] = {
 	&omnia_mcu_base_group,
 	&omnia_mcu_gpio_group,
+	&omnia_mcu_poweroff_group,
 	NULL
 };
 
@@ -372,6 +373,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

258 lines not on this page · raw message

drivers/platform/cznic/turris-omnia-mcu.h +20 -0
index b62afc663c07..4b49736d6d3f 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 i2c_client;
 
@@ -36,6 +38,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,
@@ -48,6 +55,17 @@ static inline int omnia_cmd_write(const struct i2c_client *client, void *cmd,
 	return omnia_cmd_write_read(client, cmd, len, NULL, 0);
 }
 
+static inline int omnia_cmd_write_u32(const struct i2c_client *client, u8 cmd,
+				      u32 val)
+{
+	u8 buf[5];
+
+	buf[0] = cmd;
+	put_unaligned_le32(val, &buf[1]);
+
+	return omnia_cmd_write(client, buf, sizeof(buf));
+}
+
 static inline int omnia_cmd_read(const struct i2c_client *client, u8 cmd,
 				 void *reply, unsigned int len)
 {
@@ -129,7 +147,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
#10 lore raw

Re: [PATCH v9 4/9] 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 130

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]);
	put_unaligned_le32(crc32_be(~0, (void *)&tmp, sizeof(tmp)), &cmd[5]);
Signature · 2 lines
With Best Regards,
Andy Shevchenko
author maintainer
#11 lore raw

PATCH v9 5/9 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         | 127 ++++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |  24 ++++
 5 files changed, 158 insertions(+)
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-watchdog.c
drivers/platform/cznic/Kconfig +2 -0
index c1e719235517..e262930b3faf 100644
@@ -19,6 +19,7 @@ config TURRIS_OMNIA_MCU
 	select GPIOLIB
 	select GPIOLIB_IRQCHIP
 	select RTC_CLASS
+	select WATCHDOG_CORE
 	help
 	  Say Y here to add support for the features implemented by the
 	  microcontroller on the CZ.NIC's Turris Omnia SOHO router.
@@ -26,6 +27,7 @@ config TURRIS_OMNIA_MCU
 	  - board poweroff into true low power mode (with voltage regulators
 	    disabled) and the ability to configure wake up from this mode (via
 	    rtcwake)
+	  - MCU watchdog
 	  - GPIO pins
 	    - to get front button press events (the front button can be
 	      configured either to generate press events to the CPU or to change
drivers/platform/cznic/Makefile +1 -0
index a185ef882e44..687f7718c0a1 100644
@@ -4,3 +4,4 @@ obj-$(CONFIG_TURRIS_OMNIA_MCU)	+= turris-omnia-mcu.o
 turris-omnia-mcu-y		:= turris-omnia-mcu-base.o
 turris-omnia-mcu-y		+= turris-omnia-mcu-gpio.o
 turris-omnia-mcu-y		+= turris-omnia-mcu-sys-off-wakeup.o
+turris-omnia-mcu-y		+= turris-omnia-mcu-watchdog.o
drivers/platform/cznic/turris-omnia-mcu-base.c +4 -0
index 32d187e12261..cf37c3a70497 100644
@@ -377,6 +377,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 +127 -0

127 lines not on this page · raw message

drivers/platform/cznic/turris-omnia-mcu.h +24 -0
index 4b49736d6d3f..a5a0b55b56d4 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>
@@ -43,6 +44,9 @@ struct omnia_mcu {
 	struct rtc_device *rtcdev;
 	u32 rtc_alarm;
 	bool front_button_poweron;
+
+	/* MCU watchdog */
+	struct watchdog_device wdt;
 };
 
 int omnia_cmd_write_read(const struct i2c_client *client,
@@ -55,6 +59,25 @@ static inline int omnia_cmd_write(const struct i2c_client *client, void *cmd,
 	return omnia_cmd_write_read(client, cmd, len, NULL, 0);
 }
 
+static inline int omnia_cmd_write_u8(const struct i2c_client *client, u8 cmd,
+				     u8 val)
+{
+	u8 buf[2] = { cmd, val };
+
+	return omnia_cmd_write(client, buf, sizeof(buf));
+}
+
+static inline int omnia_cmd_write_u16(const struct i2c_client *client, u8 cmd,
+				      u16 val)
+{
+	u8 buf[3];
+
+	buf[0] = cmd;
+	put_unaligned_le16(val, &buf[1]);
+
+	return omnia_cmd_write(client, buf, sizeof(buf));
+}
+
 static inline int omnia_cmd_write_u32(const struct i2c_client *client, u8 cmd,
 				      u32 val)
 {
@@ -151,5 +174,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.43.2
#12 lore raw

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

Quoted 1 line · Marek Behún

Add support for the watchdog mechanism provided by the MCU.

...

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

Quoted 2 lines
drivers/platform/cznic/turris-omnia-mcu-watchdog.c new file +2 -0
+	return omnia_cmd_write_u16(mcu->client, OMNIA_CMD_SET_WDT_TIMEOUT,
+				   timeout * 10);

DECI (from units.h)

...

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

Quoted 1 line
drivers/platform/cznic/turris-omnia-mcu-watchdog.c new file +1 -0
+	return timeleft / 10;

Ditto.

...

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

Quoted 1 line
drivers/platform/cznic/turris-omnia-mcu-watchdog.c new file +1 -0
+	mcu->wdt.max_timeout = 6553; /* 65535 deciseconds */

Ditto.

	mcu->wdt.max_timeout = 65536 / DECI;

(and no need in a comment)

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

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

In reply to 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/Kconfig                |   2 +
 drivers/platform/cznic/Makefile               |   1 +
 .../platform/cznic/turris-omnia-mcu-base.c    |   6 +-
 .../platform/cznic/turris-omnia-mcu-gpio.c    |   2 +-
 .../platform/cznic/turris-omnia-mcu-trng.c    | 103 ++++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |   8 ++
 6 files changed, 120 insertions(+), 2 deletions(-)
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-trng.c
drivers/platform/cznic/Kconfig +2 -0
index e262930b3faf..6edac80d5fa3 100644
@@ -18,6 +18,7 @@ config TURRIS_OMNIA_MCU
 	depends on I2C
 	select GPIOLIB
 	select GPIOLIB_IRQCHIP
+	select HW_RANDOM
 	select RTC_CLASS
 	select WATCHDOG_CORE
 	help
@@ -27,6 +28,7 @@ config TURRIS_OMNIA_MCU
 	  - board poweroff into true low power mode (with voltage regulators
 	    disabled) and the ability to configure wake up from this mode (via
 	    rtcwake)
+	  - true random number generator (if available on the MCU)
 	  - MCU watchdog
 	  - GPIO pins
 	    - to get front button press events (the front button can be
drivers/platform/cznic/Makefile +1 -0
index 687f7718c0a1..eae4c6b341ff 100644
@@ -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
drivers/platform/cznic/turris-omnia-mcu-base.c +5 -1
index cf37c3a70497..70ae5687ab4c 100644
@@ -381,7 +381,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[] = {
drivers/platform/cznic/turris-omnia-mcu-gpio.c +1 -1
index 438c8e909c5c..f1a931d6b16b 100644
@@ -167,7 +167,7 @@ static const struct omnia_gpio omnia_gpios[64] = {
 };
 
 /* mapping from interrupts to indexes of GPIOs in the omnia_gpios array */
-static const u8 omnia_int_to_gpio_idx[32] = {
+const u8 omnia_int_to_gpio_idx[32] = {
 	[__bf_shf(OMNIA_INT_CARD_DET)]			= 4,
 	[__bf_shf(OMNIA_INT_MSATA_IND)]			= 5,
 	[__bf_shf(OMNIA_INT_USB30_OVC)]			= 6,
drivers/platform/cznic/turris-omnia-mcu-trng.c new file +103 -0

103 lines not on this page · raw message

drivers/platform/cznic/turris-omnia-mcu.h +8 -0
index a5a0b55b56d4..ef0b7dd6e485 100644
@@ -9,7 +9,9 @@
 #define __TURRIS_OMNIA_MCU_H
 
 #include <linux/bitops.h>
+#include <linux/completion.h>
 #include <linux/gpio/driver.h>
+#include <linux/hw_random.h>
 #include <linux/if_ether.h>
 #include <linux/mutex.h>
 #include <linux/rtc.h>
@@ -47,6 +49,10 @@ struct omnia_mcu {
 
 	/* MCU watchdog */
 	struct watchdog_device wdt;
+
+	/* true random number generator */
+	struct hwrng trng;
+	struct completion trng_completion;
 };
 
 int omnia_cmd_write_read(const struct i2c_client *client,
@@ -169,11 +175,13 @@ static inline int omnia_cmd_read_u8(const struct i2c_client *client, u8 cmd,
 	return omnia_cmd_read(client, cmd, reply, sizeof(*reply));
 }
 
+extern const u8 omnia_int_to_gpio_idx[32];
 extern const struct attribute_group omnia_mcu_gpio_group;
 extern const struct attribute_group omnia_mcu_poweroff_group;
 
 int omnia_mcu_register_gpiochip(struct omnia_mcu *mcu);
 int omnia_mcu_register_sys_off_and_wakeup(struct omnia_mcu *mcu);
+int omnia_mcu_register_trng(struct omnia_mcu *mcu);
 int omnia_mcu_register_watchdog(struct omnia_mcu *mcu);
 
 #endif /* __TURRIS_OMNIA_MCU_H */
Signature · 1 line
2.43.2
author maintainer
#14 lore raw

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

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

---
 .../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 | 207 ++++++++++++++++++
 drivers/platform/cznic/turris-omnia-mcu.h     |  23 ++
 8 files changed, 304 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/debugfs-turris-omnia-mcu
 create mode 100644 drivers/platform/cznic/turris-omnia-mcu-debugfs.c
Documentation/ABI/testing/debugfs-turris-omnia-mcu new file +13 -0
new file mode 100644
index 000000000000..1665005c2dcd
@@ -0,0 +1,13 @@
+What:		/sys/kernel/debug/turris-omnia-mcu/do_sign
+Date:		July 2024
+KernelVersion:	6.10
+Contact:	Marek Behún <[email protected]>
+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.
+		======= ===========================================================
Documentation/ABI/testing/sysfs-bus-i2c-devices-turris-omnia-mcu +13 -0
index 5e2d8ec52374..42710b8c77ef 100644
@@ -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 <[email protected]>
+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
MAINTAINERS +1 -0
index b27cc5feb64f..11c93b6029f4 100644
@@ -2139,6 +2139,7 @@ M:	Marek Behún <[email protected]>
 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
drivers/platform/cznic/Kconfig +2 -0
index 6edac80d5fa3..152c866d63a6 100644
@@ -29,6 +29,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
drivers/platform/cznic/Makefile +1 -0
index eae4c6b341ff..af9213928404 100644
@@ -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
drivers/platform/cznic/turris-omnia-mcu-base.c +44 -1

81 lines not on this page · raw message

drivers/platform/cznic/turris-omnia-mcu-debugfs.c new file +207 -0

207 lines not on this page · raw message

drivers/platform/cznic/turris-omnia-mcu.h +23 -0
index ef0b7dd6e485..4c013150a5e7 100644
@@ -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 i2c_client;
 
 struct omnia_mcu {
@@ -32,6 +35,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;
@@ -53,6 +57,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,
@@ -184,4 +198,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 */
Signature · 1 line
2.43.2
#15 lore raw

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

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

...

Quoted 10 lines
drivers/platform/cznic/turris-omnia-mcu-debugfs.c new file +10 -0
+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, OMNIA_CMD_CRYPTO_COLLECT_SIGNATURE,
+			     reply, sizeof(reply));
+	if (!err && reply[0] != OMNIA_MCU_CRYPTO_SIGNATURE_LEN)
+		err = -EIO;

drivers/platform/cznic/turris-omnia-mcu-debugfs.c line 52

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

Even for an error case?

Quoted 5 lines
drivers/platform/cznic/turris-omnia-mcu-debugfs.c new file +5 -0
+		complete(&mcu->msg_signed_completion);
+	}
+
+	return IRQ_HANDLED;
+}

...

drivers/platform/cznic/turris-omnia-mcu-debugfs.c line 102

Quoted 4 lines
drivers/platform/cznic/turris-omnia-mcu-debugfs.c new file +4 -0
+	scoped_guard(mutex, &mcu->sign_lock)
+		if (mcu->sign_state != SIGN_STATE_REQUESTED &&
+		    mcu->sign_state != SIGN_STATE_COLLECTED)
+			return -ENODATA;

{}

Don't you want interruptible mutex? In such case you might need to return -ERESTARTSYS. OTOH, this is debugfs, we don't much care.

...

Quoted 1 line

+#define OMNIA_MCU_CRYPTO_PUBLIC_KEY_LEN 33

33? Hmm... does it mean (32 + 1)?

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

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

Quoted 1 line · On Wed, 8 May 2024 14:33:12 +0300 Andy Shevchenko <[email protected]> wrote:

On Wed, May 08, 2024 at 12:31:16PM +0200, Marek Behún wrote:

...

Quoted 19 lines
Quoted 18 lines
drivers/platform/cznic/turris-omnia-mcu-debugfs.c new file +18 -0
+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, OMNIA_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);  
Quoted 3 lines

drivers/platform/cznic/turris-omnia-mcu-debugfs.c line 52

Quoted 1 line
drivers/platform/cznic/turris-omnia-mcu-debugfs.c new file +1 -0
+		mcu->sign_state = SIGN_STATE_COLLECTED;  

Even for an error case?

Yes, the pair (errno, signature) is collected.

Quoted 15 lines
Quoted 5 lines
drivers/platform/cznic/turris-omnia-mcu-debugfs.c new file +5 -0
+		complete(&mcu->msg_signed_completion);
+	}
+
+	return IRQ_HANDLED;
+}  

...

drivers/platform/cznic/turris-omnia-mcu-debugfs.c line 102

Quoted 4 lines
drivers/platform/cznic/turris-omnia-mcu-debugfs.c new file +4 -0
+	scoped_guard(mutex, &mcu->sign_lock)
+		if (mcu->sign_state != SIGN_STATE_REQUESTED &&
+		    mcu->sign_state != SIGN_STATE_COLLECTED)
+			return -ENODATA;  

{}

Quoted 2 lines

Don't you want interruptible mutex? In such case you might need to return -ERESTARTSYS. OTOH, this is debugfs, we don't much care.

Indeed I shall use

  scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &mcu->sign_lock) {
    ...

}

And -ERESTARTSYS instead of -EINTR also for the subsequent wait_for_completion_interruptible(), and also in trng from patch 6/9.

Quoted 5 lines

...

Quoted 1 line

+#define OMNIA_MCU_CRYPTO_PUBLIC_KEY_LEN 33

33? Hmm... does it mean (32 + 1)?

Rather (1 + 32), the first byte is 0x02 or 0x03, determining whether the y coordinate of the public key elliptic curve point is positive or negative, and the rest 32 bytes are the x coordinate.

Marek

#17 lore raw

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

Quoted 3 lines · Marek Behún
Quoted 1 line · On Wed, 8 May 2024 14:33:12 +0300 Andy Shevchenko <[email protected]> wrote:

On Wed, May 08, 2024 at 12:31:16PM +0200, Marek Behún wrote:

...

Quoted 7 lines
Quoted 3 lines
Quoted 1 line

+#define OMNIA_MCU_CRYPTO_PUBLIC_KEY_LEN 33

33? Hmm... does it mean (32 + 1)?

Rather (1 + 32), the first byte is 0x02 or 0x03, determining whether the y coordinate of the public key elliptic curve point is positive or negative, and the rest 32 bytes are the x coordinate.

I hope you used that in v10, i.e. (1 + 32) with a summary of the above in the comment.

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

PATCH v9 8/9 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.

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

---
 .../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
author maintainer
#19 lore raw

PATCH v9 9/9 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.

---
 .../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
#20 lore raw
In reply to Marek Behún
Quoted 8 lines · Marek Behún

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

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

This series still depends on the immutable branch between LEDs and locking, introducing devm_mutex_init(), see the PR
https://lore.kernel.org/linux-leds/[email protected]/

Quoted 9 lines

See also cover letters for v1, v2, v3, v4, v5, v6, v7 and v8:
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]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/

From GPIO implementation perspective, it's good enough in my opinion. The rest can be amended later on.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
author maintainer
#21 lore raw
Quoted 19 lines · Andy Shevchenko
Quoted 8 lines · Marek Behún

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

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

This series still depends on the immutable branch between LEDs and locking, introducing devm_mutex_init(), see the PR
https://lore.kernel.org/linux-leds/[email protected]/

Quoted 9 lines

See also cover letters for v1, v2, v3, v4, v5, v6, v7 and v8:
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]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/
https://patchwork.kernel.org/project/linux-soc/cover/[email protected]/

Quoted 2 lines

From GPIO implementation perspective, it's good enough in my opinion. The rest can be amended later on.

I will send v10 tomorrow with these issues fixed. We'll see if Arnd will be willing to take this for 6.10, and if not, 6.11 will it be.

Marek