* [PATCH v2 0/8] mfd: Add support for NXP MC33978/MC34978 MSDI
@ 2026-03-03 13:39 Oleksij Rempel
2026-03-03 13:39 ` [PATCH v2 1/6] dt-bindings: mfd: add " Oleksij Rempel
` (5 more replies)
0 siblings, 6 replies; 25+ messages in thread
From: Oleksij Rempel @ 2026-03-03 13:39 UTC (permalink / raw)
To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lee Jones, Peter Rosin, Linus Walleij
Cc: Oleksij Rempel, kernel, linux-kernel, devicetree, linux-hwmon,
linux-gpio, David Jander
This series adds support for the NXP MC33978/MC34978 Multiple Switch Detection
Interface (MSDI) via the MFD framework.
Architecture overview:
* mfd: Core driver handling 2-frame pipelined SPI, regulator sequencing, and
linear irq_domain. Harvests status bits from SPI MISO MSB.
* pinctrl: Exposes 22 physical switch inputs as standard GPIOs. Proxies IRQs to
the MFD domain.
* hwmon: Exposes thermal limits, VBATP/VDDQ voltage boundaries, and dynamic
fault alarms.
* mux: Controls the 24-to-1 AMUX routing analog signals (switch voltages,
temperature, VBATP) to an external ADC.
Initial pinctrl implementation by David Jander, reworked into this MFD
architecture.
Best regards,
Oleksij
David Jander (1):
pinctrl: add NXP MC33978/MC34978 pinctrl driver
Oleksij Rempel (5):
dt-bindings: mfd: add NXP MC33978/MC34978 MSDI
mfd: add NXP MC33978/MC34978 core driver
pinctrl: core: Make pin group callbacks optional
hwmon: add NXP MC33978/MC34978 driver
mux: add NXP MC33978/MC34978 AMUX driver
.../devicetree/bindings/mfd/nxp,mc33978.yaml | 114 +++
.../bindings/pinctrl/nxp,mc33978-pinctrl.yaml | 82 ++
drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/mc33978-hwmon.c | 430 +++++++++
drivers/mfd/Kconfig | 14 +
drivers/mfd/Makefile | 2 +
drivers/mfd/mc33978.c | 832 ++++++++++++++++++
drivers/mux/Kconfig | 14 +
drivers/mux/Makefile | 2 +
drivers/mux/mc33978-mux.c | 120 +++
drivers/pinctrl/Kconfig | 14 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/core.c | 25 +-
drivers/pinctrl/pinconf.c | 18 +-
drivers/pinctrl/pinctrl-mc33978.c | 733 +++++++++++++++
include/linux/mfd/mc33978.h | 86 ++
17 files changed, 2488 insertions(+), 10 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/nxp,mc33978.yaml
create mode 100644 Documentation/devicetree/bindings/pinctrl/nxp,mc33978-pinctrl.yaml
create mode 100644 drivers/hwmon/mc33978-hwmon.c
create mode 100644 drivers/mfd/mc33978.c
create mode 100644 drivers/mux/mc33978-mux.c
create mode 100644 drivers/pinctrl/pinctrl-mc33978.c
create mode 100644 include/linux/mfd/mc33978.h
--
2.47.3
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 1/6] dt-bindings: mfd: add NXP MC33978/MC34978 MSDI
2026-03-03 13:39 [PATCH v2 0/8] mfd: Add support for NXP MC33978/MC34978 MSDI Oleksij Rempel
@ 2026-03-03 13:39 ` Oleksij Rempel
2026-03-03 14:40 ` Rob Herring (Arm)
2026-03-04 7:59 ` Krzysztof Kozlowski
2026-03-03 13:39 ` [PATCH v2 2/6] mfd: add NXP MC33978/MC34978 core driver Oleksij Rempel
` (4 subsequent siblings)
5 siblings, 2 replies; 25+ messages in thread
From: Oleksij Rempel @ 2026-03-03 13:39 UTC (permalink / raw)
To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lee Jones, Peter Rosin, Linus Walleij
Cc: Oleksij Rempel, kernel, linux-kernel, devicetree, linux-hwmon,
linux-gpio, David Jander
Add device tree binding documentation for the NXP MC33978 and MC34978
Multiple Switch Detection Interface (MSDI) devices.
These ICs monitor up to 22 mechanical switch contacts in automotive and
industrial environments. They provide configurable wetting currents to
break through contact oxidation and feature extensive hardware
protection against thermal overload and voltage transients (load
dumps/brown-outs).
The device interfaces via SPI and provides multiple functions. To
accurately represent the hardware without unnecessary DT overhead, the
binding is structured as follows:
- pinctrl: A dedicated child node managing the 22 switch inputs (SG/SP
pins) and their GPIO configurations.
- hwmon: Integrated into the parent node, exposing critical hardware
faults (OT, OV, UV) and static voltage/temperature thresholds.
- mux: Integrated into the parent node, controlling the 24-to-1 analog
multiplexer to route pin voltages, internal temperature, or battery
voltage to an external SoC ADC.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v2:
- Squashed MFD, pinctrl, hwmon, and mux bindings into a single patch
- Removed the empty hwmon child node
- Folded the mux-controller node into the parent MFD node
- Added vbatp-supply and vddq-supply to the required properties block
- Changed the example node name from mc33978@0 to gpio@0
- Removed unnecessary literal block scalars (|) from descriptions
- Documented SG, SP, and SB pin acronyms in the pinctrl description
- Added consumer polarity guidance (GPIO_ACTIVE_LOW/HIGH) for SG/SB
inputs, with a note on output circuit dependency
- Updated commit message
---
.../devicetree/bindings/mfd/nxp,mc33978.yaml | 114 ++++++++++++++++++
.../bindings/pinctrl/nxp,mc33978-pinctrl.yaml | 82 +++++++++++++
2 files changed, 196 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/nxp,mc33978.yaml
create mode 100644 Documentation/devicetree/bindings/pinctrl/nxp,mc33978-pinctrl.yaml
diff --git a/Documentation/devicetree/bindings/mfd/nxp,mc33978.yaml b/Documentation/devicetree/bindings/mfd/nxp,mc33978.yaml
new file mode 100644
index 000000000000..9deb5de0ce23
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/nxp,mc33978.yaml
@@ -0,0 +1,114 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/nxp,mc33978.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP MC33978/MC34978 Multiple Switch Detection Interface
+
+maintainers:
+ - David Jander <david@protonic.nl>
+ - Oleksij Rempel <o.rempel@pengutronix.de>
+
+description:
+ The MC33978 and MC34978 are Multiple Switch Detection Interface (MSDI)
+ devices with 22 switch inputs, integrated fault detection, and analog
+ multiplexer (AMUX) for voltage/temperature monitoring.
+
+allOf:
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+ $nodename:
+ pattern: "^gpio(@.*)?$"
+
+ compatible:
+ enum:
+ - nxp,mc33978
+ - nxp,mc34978
+
+ reg:
+ maxItems: 1
+ description: SPI chip select number
+
+ spi-max-frequency:
+ maximum: 8000000
+ description: Maximum SPI clock frequency (up to 8 MHz)
+
+ interrupts:
+ maxItems: 1
+ description:
+ INT_B pin interrupt. Active-low, indicates pin state changes or
+ fault conditions.
+
+ interrupt-controller: true
+
+ '#interrupt-cells':
+ const: 2
+ description:
+ First cell is the IRQ number (0-21 for pins, 22 for faults).
+ Second cell is the trigger type (IRQ_TYPE_* from interrupt-controller.h).
+
+ '#mux-control-cells':
+ const: 0
+ description:
+ Present if the device AMUX selector is used as a mux provider.
+ Consumers (e.g. io-channel-mux) must provide settle-time-us for the
+ external ADC sampling path.
+
+ vddq-supply:
+ description: Digital supply voltage
+
+ vbatp-supply:
+ description: Battery/power supply
+
+patternProperties:
+ "^pinctrl(@.*)?":
+ type: object
+ $ref: /schemas/pinctrl/nxp,mc33978-pinctrl.yaml#
+ description:
+ Pinctrl and GPIO controller child node for the 22 switch inputs.
+
+required:
+ - compatible
+ - interrupt-controller
+ - '#interrupt-cells'
+ - interrupts
+ - reg
+ - vbatp-supply
+ - vddq-supply
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ msdi: gpio@0 {
+ compatible = "nxp,mc33978";
+ reg = <0>;
+ spi-max-frequency = <4000000>;
+
+ interrupt-parent = <&gpiog>;
+ interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+
+ vddq-supply = <®_3v3>;
+ vbatp-supply = <®_12v>;
+
+ #mux-control-cells = <0>;
+
+ pinctrl {
+ compatible = "nxp,mc33978-pinctrl";
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+
+ };
+ };
diff --git a/Documentation/devicetree/bindings/pinctrl/nxp,mc33978-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nxp,mc33978-pinctrl.yaml
new file mode 100644
index 000000000000..f8257d55d466
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/nxp,mc33978-pinctrl.yaml
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/nxp,mc33978-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP MC33978/MC34978 Pinctrl/GPIO Driver
+
+maintainers:
+ - David Jander <david@protonic.nl>
+ - Oleksij Rempel <o.rempel@pengutronix.de>
+
+description: |
+ Pin control and GPIO driver for the MC33978/MC34978 MSDI device.
+
+ Pin numbering:
+ - Pins 0-13: SG0-SG13 (Switch-to-Ground inputs). These pins monitor
+ contacts closed to ground and typically require GPIO_ACTIVE_LOW
+ flags when used as digital inputs.
+ - Pins 14-21: SP0-SP7 (Programmable inputs). These can be configured
+ as SG (Switch-to-Ground) or SB (Switch-to-Battery) inputs. SB
+ inputs monitor contacts closed to the battery voltage and typically
+ require GPIO_ACTIVE_HIGH flags when used as digital inputs.
+
+ Output Emulation:
+ The hardware lacks standard push-pull output drivers. Outputs are emulated
+ by toggling the programmable wetting current sources (acting as pull-ups or
+ pull-downs) and the hardware tri-state registers. Because of this physical
+ constraint:
+ - Consumers using pins as outputs MUST flag them with GPIO_OPEN_DRAIN or
+ GPIO_OPEN_SOURCE in the device tree.
+ - Push-pull configurations are physically unsupported.
+ - The active polarity depends entirely on the external circuit (e.g., how an
+ LED is wired) and must be flagged accordingly by the consumer.
+
+properties:
+ compatible:
+ enum:
+ - nxp,mc33978-pinctrl
+ - nxp,mc34978-pinctrl
+
+ gpio-controller: true
+
+ '#gpio-cells':
+ const: 2
+
+ ngpios:
+ const: 22
+
+patternProperties:
+ "^.*-grp$":
+ type: object
+ $ref: pincfg-node.yaml#
+ additionalProperties: false
+ description:
+ Pin configuration subnodes.
+ properties:
+ pins: true
+ bias-pull-up: true
+ bias-pull-down: true
+ bias-high-impedance: true
+
+required:
+ - compatible
+ - gpio-controller
+ - '#gpio-cells'
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ pinctrl {
+ compatible = "nxp,mc33978-pinctrl";
+ gpio-controller;
+ #gpio-cells = <2>;
+ ngpios = <22>;
+
+ door-grp {
+ pins = "sg0";
+ bias-high-impedance;
+ };
+ };
--
2.47.3
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 2/6] mfd: add NXP MC33978/MC34978 core driver
2026-03-03 13:39 [PATCH v2 0/8] mfd: Add support for NXP MC33978/MC34978 MSDI Oleksij Rempel
2026-03-03 13:39 ` [PATCH v2 1/6] dt-bindings: mfd: add " Oleksij Rempel
@ 2026-03-03 13:39 ` Oleksij Rempel
2026-03-03 13:39 ` [PATCH v2 3/6] pinctrl: core: Make pin group callbacks optional Oleksij Rempel
` (3 subsequent siblings)
5 siblings, 0 replies; 25+ messages in thread
From: Oleksij Rempel @ 2026-03-03 13:39 UTC (permalink / raw)
To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lee Jones, Peter Rosin, Linus Walleij
Cc: Oleksij Rempel, kernel, linux-kernel, devicetree, linux-hwmon,
linux-gpio, David Jander
Add core Multi-Function Device (MFD) driver for the NXP MC33978 and
MC34978 Multiple Switch Detection Interfaces (MSDI).
The MC33978/MC34978 devices provide 22 switch detection inputs, analog
multiplexing (AMUX), and comprehensive hardware fault detection.
This core driver handles:
- SPI communications via a custom regmap bus to support the device's
pipelined two-frame MISO response requirement.
- Power sequencing for the VDDQ (logic) and VBATP (battery) regulators.
- Interrupt demultiplexing, utilizing an irq_domain to provide 22 virtual
IRQs for switch state changes and 1 virtual IRQ for hardware faults.
- Inline status harvesting from the SPI MSB to detect and trigger events
without requiring dedicated status register polling.
Child devices (pinctrl, hwmon, mux) are populated automatically via
the device tree.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v2:
- Rewrite the driver header comment
- Explicitly reject IRQ_TYPE_LEVEL_HIGH and IRQ_TYPE_LEVEL_LOW in
mc33978_irq_set_type() to correctly reflect the hardware's edge-only
interrupt capabilities.
- Pass the hardware fault IRQ to the hwmon child driver via mfd_cell
resources, rather than requiring the child to parse the parent's irq_domain.
- Ensure the Kconfig strictly depends on OF and SPI
---
drivers/mfd/Kconfig | 14 +
drivers/mfd/Makefile | 2 +
drivers/mfd/mc33978.c | 832 ++++++++++++++++++++++++++++++++++++
include/linux/mfd/mc33978.h | 86 ++++
4 files changed, 934 insertions(+)
create mode 100644 drivers/mfd/mc33978.c
create mode 100644 include/linux/mfd/mc33978.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 7192c9d1d268..f98469291bbd 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2566,6 +2566,20 @@ config MFD_UPBOARD_FPGA
To compile this driver as a module, choose M here: the module will be
called upboard-fpga.
+config MFD_MC33978
+ tristate "NXP MC33978/MC34978 industrial input controller core"
+ depends on OF
+ depends on SPI
+ select MFD_CORE
+ select REGMAP
+ help
+ Support for the NXP MC33978/MC34978 industrial input controllers
+ using the SPI interface.
+
+ This driver provides common support for accessing the device.
+ Additional drivers must be enabled in order to use the functionality
+ of the device.
+
config MFD_MAX7360
tristate "Maxim MAX7360 I2C IO Expander"
depends on I2C
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index e75e8045c28a..dcd99315f683 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -122,6 +122,8 @@ obj-$(CONFIG_MFD_MC13XXX) += mc13xxx-core.o
obj-$(CONFIG_MFD_MC13XXX_SPI) += mc13xxx-spi.o
obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
+obj-$(CONFIG_MFD_MC33978) += mc33978.o
+
obj-$(CONFIG_MFD_PF1550) += pf1550.o
obj-$(CONFIG_MFD_NCT6694) += nct6694.o
diff --git a/drivers/mfd/mc33978.c b/drivers/mfd/mc33978.c
new file mode 100644
index 000000000000..df4d55114591
--- /dev/null
+++ b/drivers/mfd/mc33978.c
@@ -0,0 +1,832 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024 David Jander <david@protonic.nl>, Protonic Holland
+ * Copyright (C) 2026 Oleksij Rempel <kernel@pengutronix.de>, Pengutronix
+ *
+ * MC33978/MC34978 Multiple Switch Detection Interface - MFD Core Driver
+ *
+ * Driver Architecture:
+ * This is the core MFD driver handling the physical SPI interface, power
+ * management, and central interrupt routing. It instantiates the following
+ * child devices:
+ * - pinctrl: For GPIO read/write and wetting current configuration.
+ * - hwmon: For hardware fault monitoring (tLIM, over/under-voltage).
+ * - mux: For the 24-to-1 analog multiplexer (AMUX).
+ *
+ * Custom SPI Regmap & Event Harvesting:
+ * The device uses a non-standard pipelined SPI protocol where the MISO
+ * response logically lags the MOSI command by one frame. Furthermore, the
+ * hardware embeds volatile global status bits (INT_flg, FAULT_STAT) into the
+ * high byte of almost every SPI response (with specific exceptions handled by
+ * the decoder). This core implements a custom regmap_bus to handle the
+ * 2-frame dummy fetches and transparently "harvests" these status bits in
+ * the background to schedule event processing.
+ *
+ * Interrupt Quirks & Limitations:
+ * - Clear-on-Read: The physical INT_B line is directly tied to the INT_flg
+ * bit. The hardware deasserts INT_B immediately upon *any* SPI transfer
+ * that returns INT_flg. Harvesting this bit from all SPI traffic is the
+ * ONLY way to know this device triggered an interrupt (crucial for shared
+ * IRQ lines).
+ * - Stateless Pin Edge Detection: The hardware lacks per-pin interrupt status
+ * registers. To determine which pin triggered an event, the driver must
+ * read the current pin states and XOR them against a previously cached state.
+ * - Missed Short Pulses: Because pin interrupts are state-derived rather than
+ * hardware-latched, very short physical pulses (shorter than the SPI read
+ * latency) will be missed entirely if the pin reverts to its original state
+ * before the READ_IN register is sampled by the IRQ thread.
+ * - Edge-Only Pin Interrupts: The hardware only asserts INT_B on a state
+ * change. It cannot continuously assert an interrupt while a pin is held at a
+ * specific logic level. Consequently, the driver strictly emulates edge
+ * interrupts (RISING/FALLING) and explicitly rejects LEVEL interrupt
+ * configurations to prevent consumer misalignment.
+ */
+
+#include <linux/array_size.h>
+#include <linux/atomic.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/device.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/mfd/core.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/string.h>
+
+#include <linux/mfd/mc33978.h>
+
+#define MC33978_DRV_NAME "mc33978"
+
+/* Device identification signature returned by CHECK register */
+#define MC33978_CHECK_SIGNATURE 0x123456
+
+/*
+ * Pipelined two-frame SPI transfer:
+ * [REQ] - Transmits command/write-data, receives dummy/previous response
+ * [PIPE] - Transmits dummy CHECK, receives actual response to current command
+ */
+enum mc33978_frame_index {
+ MC33978_FRAME_REQ = 0,
+ MC33978_FRAME_PIPE,
+ MC33978_FRAME_COUNT
+};
+
+/* SPI frame byte offsets (transmitted MSB first) */
+enum mc33978_frame_offset {
+ MC33978_FRAME_CMD = 0,
+ MC33978_FRAME_DATA_HI,
+ MC33978_FRAME_DATA_MID,
+ MC33978_FRAME_DATA_LO
+};
+
+#define MC33978_FRAME_LEN 4
+
+/* Regmap internal value buffer offsets */
+enum mc33978_payload_offset {
+ MC33978_PAYLOAD_HI = 0,
+ MC33978_PAYLOAD_MID,
+ MC33978_PAYLOAD_LO
+};
+
+#define MC33978_PAYLOAD_LEN 3
+
+/*
+ * SPI Command Byte (FRAME_CMD).
+ * Maps to frame bit [24] in the datasheet.
+ */
+#define MC33978_CMD_BYTE_WRITE BIT(0)
+
+/* High Payload Byte Masks (FRAME_DATA_HI / PAYLOAD_HI). */
+#define MC33978_HI_BYTE_STAT_FAULT BIT(7) /* Maps to frame bit [23] */
+#define MC33978_HI_BYTE_STAT_INT BIT(6) /* Maps to frame bit [22] */
+
+#define MC33978_HI_BYTE_STATUS_MASK (MC33978_HI_BYTE_STAT_FAULT | \
+ MC33978_HI_BYTE_STAT_INT)
+
+/* Maps to frame bits [21:16] */
+#define MC33978_HI_BYTE_DATA_MASK GENMASK(5, 0)
+
+#define MC33978_CACHE_SG_PIN_MASK GENMASK(13, 0)
+#define MC33978_CACHE_SP_PIN_MASK GENMASK(21, 14)
+
+#define MC33978_SG_PIN_MASK GENMASK(13, 0)
+#define MC33978_SP_PIN_MASK GENMASK(7, 0)
+
+struct mc33978_data {
+ const struct mfd_cell *cells;
+ int num_cells;
+};
+
+struct mc33978_mfd_priv {
+ struct spi_device *spi;
+ struct regmap *map;
+
+ struct regulator *vddq;
+ struct regulator *vbatp;
+
+ /* Protects event processing loop and hardware state machine */
+ struct mutex event_lock;
+ struct work_struct event_work;
+ atomic_t is_handling;
+ atomic_t harvested_flags;
+ u32 cached_pin_state;
+ u32 cached_pin_mask;
+ u32 irq_rise;
+ u32 irq_fall;
+
+ /* Protects IRQ mask registers and cached IRQ state */
+ struct mutex irq_lock;
+ struct irq_domain *domain;
+
+ /* Pre-built SPI messages */
+ struct spi_message msg_read;
+ struct spi_message msg_write;
+ struct spi_transfer xfer_read[MC33978_FRAME_COUNT];
+ struct spi_transfer xfer_write;
+
+ /* DMA buffers at the end */
+ u8 tx_frame[MC33978_FRAME_COUNT][MC33978_FRAME_LEN] ____cacheline_aligned;
+ u8 rx_frame[MC33978_FRAME_COUNT][MC33978_FRAME_LEN];
+};
+
+static void mc33978_irq_mask(struct irq_data *data)
+{
+ struct mc33978_mfd_priv *mc = irq_data_get_irq_chip_data(data);
+ irq_hw_number_t hwirq = irqd_to_hwirq(data);
+
+ mc->cached_pin_mask &= ~BIT(hwirq);
+}
+
+static void mc33978_irq_unmask(struct irq_data *data)
+{
+ struct mc33978_mfd_priv *mc = irq_data_get_irq_chip_data(data);
+ irq_hw_number_t hwirq = irqd_to_hwirq(data);
+
+ mc->cached_pin_mask |= BIT(hwirq);
+}
+
+static void mc33978_irq_bus_lock(struct irq_data *data)
+{
+ struct mc33978_mfd_priv *mc = irq_data_get_irq_chip_data(data);
+
+ mutex_lock(&mc->irq_lock);
+}
+
+/**
+ * mc33978_irq_bus_sync_unlock() - Sync cached IRQ mask to hardware and unlock
+ * @data: IRQ data
+ *
+ * Writes the cached interrupt mask to the hardware IE_SG and IE_SP registers,
+ * then releases the IRQ lock. This is where the actual hardware update occurs
+ * after mask/unmask operations.
+ */
+static void mc33978_irq_bus_sync_unlock(struct irq_data *data)
+{
+ struct mc33978_mfd_priv *mc = irq_data_get_irq_chip_data(data);
+ u32 sg_mask, sp_mask;
+ int ret;
+
+ /*
+ * Split the cached 22-bit pin mask into hardware register format:
+ * - SG pins: bits [13:0] (14 pins, mask 0x3FFF)
+ * - SP pins: bits [21:14] (8 pins, mask 0xFF)
+ */
+ sg_mask = FIELD_GET(MC33978_CACHE_SG_PIN_MASK, mc->cached_pin_mask);
+ sp_mask = FIELD_GET(MC33978_CACHE_SP_PIN_MASK, mc->cached_pin_mask);
+
+ ret = regmap_update_bits(mc->map, MC33978_REG_IE_SG,
+ MC33978_SG_PIN_MASK, sg_mask);
+ if (ret)
+ goto unlock;
+
+ ret = regmap_update_bits(mc->map, MC33978_REG_IE_SP,
+ MC33978_SP_PIN_MASK, sp_mask);
+unlock:
+ if (ret)
+ dev_err(&mc->spi->dev, "Failed to sync IRQ mask to hardware: %d\n",
+ ret);
+
+ mutex_unlock(&mc->irq_lock);
+}
+
+static int mc33978_irq_set_type(struct irq_data *data, unsigned int type)
+{
+ struct mc33978_mfd_priv *mc = irq_data_get_irq_chip_data(data);
+ irq_hw_number_t hwirq = irqd_to_hwirq(data);
+ u32 mask = BIT(hwirq);
+
+ if (hwirq == MC33978_HWIRQ_FAULT)
+ return 0;
+
+ if (type & (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW))
+ return -EINVAL;
+
+ if (type & IRQ_TYPE_EDGE_RISING)
+ mc->irq_rise |= mask;
+ else
+ mc->irq_rise &= ~mask;
+
+ if (type & IRQ_TYPE_EDGE_FALLING)
+ mc->irq_fall |= mask;
+ else
+ mc->irq_fall &= ~mask;
+
+ return 0;
+}
+
+static struct irq_chip mc33978_irq_chip = {
+ .name = MC33978_DRV_NAME,
+ .irq_mask = mc33978_irq_mask,
+ .irq_unmask = mc33978_irq_unmask,
+ .irq_bus_lock = mc33978_irq_bus_lock,
+ .irq_bus_sync_unlock = mc33978_irq_bus_sync_unlock,
+ .irq_set_type = mc33978_irq_set_type,
+};
+
+static int mc33978_irq_map(struct irq_domain *d, unsigned int virq,
+ irq_hw_number_t hw)
+{
+ struct mc33978_mfd_priv *mc = d->host_data;
+
+ irq_set_chip_data(virq, mc);
+ irq_set_chip_and_handler(virq, &mc33978_irq_chip, handle_simple_irq);
+
+ irq_set_nested_thread(virq, 1);
+ irq_clear_status_flags(virq, IRQ_NOREQUEST | IRQ_NOPROBE);
+
+ return 0;
+}
+
+static const struct irq_domain_ops mc33978_irq_domain_ops = {
+ .map = mc33978_irq_map,
+ .xlate = irq_domain_xlate_twocell,
+};
+
+static void mc33978_irq_domain_remove(void *data)
+{
+ struct irq_domain *domain = data;
+
+ irq_domain_remove(domain);
+}
+
+static bool mc33978_handle_pin_changes(struct mc33978_mfd_priv *mc,
+ unsigned int pin_state)
+{
+ u32 fired_pins = 0;
+ u32 changed_pins;
+ int i;
+
+ changed_pins = pin_state ^ mc->cached_pin_state;
+ if (!changed_pins)
+ return false;
+
+ mc->cached_pin_state = pin_state;
+ changed_pins &= mc->cached_pin_mask;
+
+ if (!changed_pins)
+ return false;
+
+ fired_pins |= (changed_pins & pin_state) & mc->irq_rise;
+ fired_pins |= (changed_pins & ~pin_state) & mc->irq_fall;
+
+ for_each_set_bit(i, (unsigned long *)&fired_pins, MC33978_NUM_PINS) {
+ int virq = irq_find_mapping(mc->domain, i);
+
+ handle_nested_irq(virq);
+ }
+
+ return true;
+}
+
+static bool mc33978_handle_fault_condition(struct mc33978_mfd_priv *mc)
+{
+ int virq;
+
+ virq = irq_find_mapping(mc->domain, MC33978_HWIRQ_FAULT);
+ if (virq > 0)
+ handle_nested_irq(virq);
+
+ return true;
+}
+
+static bool mc33978_process_single_event(struct mc33978_mfd_priv *mc)
+{
+ unsigned int pin_state;
+ bool handled = false;
+ u8 hw_flags;
+ int ret;
+
+ ret = regmap_read(mc->map, MC33978_REG_READ_IN, &pin_state);
+ if (ret)
+ return false;
+
+ /*
+ * harvested_flags will be set by regmap_read() above if the FAULT_STAT
+ * or INT_flg bits were detected in the response
+ */
+ hw_flags = atomic_xchg(&mc->harvested_flags, 0);
+
+ if (mc33978_handle_pin_changes(mc, pin_state))
+ handled = true;
+
+ if (hw_flags & MC33978_HI_BYTE_STAT_FAULT &&
+ mc33978_handle_fault_condition(mc))
+ handled = true;
+
+ if (hw_flags & MC33978_HI_BYTE_STAT_INT)
+ handled = true;
+
+ return handled;
+}
+
+static bool mc33978_handle_events(struct mc33978_mfd_priv *mc)
+{
+ bool handled = false;
+
+ mutex_lock(&mc->event_lock);
+
+ do {
+ atomic_set(&mc->is_handling, 1);
+
+ if (mc33978_process_single_event(mc))
+ handled = true;
+
+ atomic_set(&mc->is_handling, 0);
+
+ } while (atomic_read(&mc->harvested_flags) != 0);
+
+ mutex_unlock(&mc->event_lock);
+
+ return handled;
+}
+
+static irqreturn_t mc33978_irq_thread(int irq, void *data)
+{
+ return mc33978_handle_events(data) ? IRQ_HANDLED : IRQ_NONE;
+}
+
+static int mc33978_irq_init(struct mc33978_mfd_priv *mc)
+{
+ struct device *dev = &mc->spi->dev;
+ int ret;
+
+ mutex_init(&mc->irq_lock);
+
+ /*
+ * Create IRQ domain with 23 interrupts:
+ * - hwirq 0-21: Pin change interrupts (22 pins)
+ * - hwirq 22: Fault interrupt (for hwmon driver)
+ */
+ mc->domain = irq_domain_add_linear(dev->of_node, MC33978_NUM_PINS + 1,
+ &mc33978_irq_domain_ops, mc);
+ if (!mc->domain)
+ return dev_err_probe(dev, -ENOMEM, "Failed to create IRQ domain\n");
+
+ ret = devm_add_action_or_reset(dev, mc33978_irq_domain_remove,
+ mc->domain);
+ if (ret)
+ return ret;
+
+ if (mc->spi->irq <= 0)
+ return dev_err_probe(dev, -EINVAL, "No valid IRQ provided for INT_B pin\n");
+
+ ret = devm_request_threaded_irq(dev, mc->spi->irq, NULL,
+ mc33978_irq_thread,
+ IRQF_ONESHOT | IRQF_SHARED,
+ dev_name(dev), mc);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to request IRQ\n");
+
+ return 0;
+}
+
+static void mc33978_event_work(struct work_struct *work)
+{
+ struct mc33978_mfd_priv *mc =
+ container_of(work, struct mc33978_mfd_priv, event_work);
+
+ mc33978_handle_events(mc);
+}
+
+/**
+ * mc33978_harvest_status() - Collect status flags from SPI responses
+ * @mc: Device private data
+ * @status: Status bits (FAULT_STAT and INT_flg) from MISO frame
+ *
+ * Accumulates status flags harvested from SPI responses and schedules
+ * event processing if not already in progress. Called by the SPI
+ * read/write functions when status bits are detected in responses.
+ */
+static void mc33978_harvest_status(struct mc33978_mfd_priv *mc, u8 status)
+{
+ if (!status)
+ return;
+
+ atomic_or(status, &mc->harvested_flags);
+
+ if (!atomic_read(&mc->is_handling))
+ schedule_work(&mc->event_work);
+}
+
+/**
+ * mc33978_prepare_messages() - Initialize the persistent SPI messages
+ * @mc: Device private data
+ *
+ * Hardware pipelining constraints:
+ * - Write (1 Frame): The device executes write commands immediately upon
+ * CS de-assertion. No fetch frame is required.
+ * - Read (2 Frames): The MISO response logically lags by one frame.
+ * Frame 1 transmits the read request and toggles CS to latch it.
+ * Frame 2 transmits a dummy CHECK command to fetch the actual payload.
+ */
+static void mc33978_prepare_messages(struct mc33978_mfd_priv *mc)
+{
+ /* --- Prepare Write Message (1 Frame) --- */
+ spi_message_init(&mc->msg_write);
+
+ mc->xfer_write.tx_buf = mc->tx_frame[MC33978_FRAME_REQ];
+ mc->xfer_write.rx_buf = mc->rx_frame[MC33978_FRAME_REQ];
+ mc->xfer_write.len = MC33978_FRAME_LEN;
+
+ spi_message_add_tail(&mc->xfer_write, &mc->msg_write);
+
+ /* --- Prepare Read Message (2 Frames) --- */
+ spi_message_init(&mc->msg_read);
+
+ /* Frame 1: Request */
+ mc->xfer_read[MC33978_FRAME_REQ].tx_buf =
+ mc->tx_frame[MC33978_FRAME_REQ];
+ mc->xfer_read[MC33978_FRAME_REQ].rx_buf =
+ mc->rx_frame[MC33978_FRAME_REQ];
+ mc->xfer_read[MC33978_FRAME_REQ].len = MC33978_FRAME_LEN;
+ mc->xfer_read[MC33978_FRAME_REQ].cs_change = 1; /* Latch command */
+
+ /* Frame 2: Fetch (Dummy CHECK) */
+ mc->xfer_read[MC33978_FRAME_PIPE].tx_buf =
+ mc->tx_frame[MC33978_FRAME_PIPE];
+ mc->xfer_read[MC33978_FRAME_PIPE].rx_buf =
+ mc->rx_frame[MC33978_FRAME_PIPE];
+ mc->xfer_read[MC33978_FRAME_PIPE].len = MC33978_FRAME_LEN;
+
+ /* Preload the dummy CHECK command statically */
+ mc->tx_frame[MC33978_FRAME_PIPE][MC33978_FRAME_CMD] = MC33978_REG_CHECK;
+
+ spi_message_add_tail(&mc->xfer_read[MC33978_FRAME_REQ], &mc->msg_read);
+ spi_message_add_tail(&mc->xfer_read[MC33978_FRAME_PIPE], &mc->msg_read);
+}
+
+/**
+ * mc33978_rx_decode() - Decode MISO response frame and extract status
+ * @rx_frame: Received SPI frame buffer (4 bytes)
+ * @val_buf: Output buffer for regmap (exactly 3 bytes, optional)
+ *
+ * Translates the 4-byte SPI response into a 3-byte regmap payload.
+ * Harvests the volatile INTflg and FAULT_STAT bits from the MSB.
+ *
+ * Return: Status bits if present, 0 otherwise.
+ */
+static u8 mc33978_rx_decode(const u8 *rx_frame, u8 *val_buf)
+{
+ u8 cmd = rx_frame[MC33978_FRAME_CMD] & ~MC33978_CMD_BYTE_WRITE;
+ bool has_status;
+ u8 status = 0;
+
+ switch (cmd) {
+ case MC33978_REG_CHECK:
+ case MC33978_REG_WET_SP:
+ case MC33978_REG_WET_SG0:
+ has_status = false;
+ break;
+ default:
+ has_status = true;
+ break;
+ }
+
+ if (has_status)
+ status = rx_frame[MC33978_FRAME_DATA_HI] &
+ MC33978_HI_BYTE_STATUS_MASK;
+
+ if (!val_buf)
+ return status;
+
+ memcpy(val_buf, &rx_frame[MC33978_FRAME_DATA_HI], MC33978_PAYLOAD_LEN);
+
+ if (has_status)
+ val_buf[MC33978_PAYLOAD_HI] &= MC33978_HI_BYTE_DATA_MASK;
+
+ return status;
+}
+
+static int mc33978_spi_write(void *ctx, const void *data, size_t count)
+{
+ struct mc33978_mfd_priv *mc = ctx;
+ u8 status;
+ int ret;
+
+ if (count != MC33978_FRAME_LEN)
+ return -EINVAL;
+
+ memcpy(mc->tx_frame[MC33978_FRAME_REQ], data, MC33978_FRAME_LEN);
+
+ ret = spi_sync(mc->spi, &mc->msg_write);
+ if (ret)
+ return ret;
+
+ status = mc33978_rx_decode(mc->rx_frame[MC33978_FRAME_REQ], NULL);
+ mc33978_harvest_status(mc, status);
+
+ return 0;
+}
+
+static int mc33978_spi_read(void *ctx, const void *reg_buf, size_t reg_size,
+ void *val_buf, size_t val_size)
+{
+ struct mc33978_mfd_priv *mc = ctx;
+ u8 status_req, status_pipe;
+ int ret;
+
+ if (reg_size != 1 || val_size != MC33978_PAYLOAD_LEN)
+ return -EINVAL;
+
+ memset(&mc->tx_frame[MC33978_FRAME_REQ][MC33978_FRAME_DATA_HI], 0,
+ MC33978_PAYLOAD_LEN);
+ mc->tx_frame[MC33978_FRAME_REQ][MC33978_FRAME_CMD] =
+ ((const u8 *)reg_buf)[0];
+
+ ret = spi_sync(mc->spi, &mc->msg_read);
+ if (ret)
+ return ret;
+
+ status_req = mc33978_rx_decode(mc->rx_frame[MC33978_FRAME_REQ], NULL);
+ status_pipe = mc33978_rx_decode(mc->rx_frame[MC33978_FRAME_PIPE],
+ val_buf);
+
+ mc33978_harvest_status(mc, status_req | status_pipe);
+
+ return 0;
+}
+
+static const struct regmap_bus mc33978_regmap_bus = {
+ .read = mc33978_spi_read,
+ .write = mc33978_spi_write,
+};
+
+static const struct regmap_range mc33978_volatile_range[] = {
+ regmap_reg_range(MC33978_REG_READ_IN, MC33978_REG_RESET),
+};
+
+static const struct regmap_access_table mc33978_volatile_table = {
+ .yes_ranges = mc33978_volatile_range,
+ .n_yes_ranges = ARRAY_SIZE(mc33978_volatile_range),
+};
+
+static const struct regmap_range mc33978_precious_range[] = {
+ regmap_reg_range(MC33978_REG_READ_IN, MC33978_REG_RESET),
+};
+
+static const struct regmap_access_table mc33978_precious_table = {
+ .yes_ranges = mc33978_precious_range,
+ .n_yes_ranges = ARRAY_SIZE(mc33978_precious_range),
+};
+
+/*
+ * NOTE: Need to fake REG_IRQ and REG_RESET as readable, so that regcache
+ * will NOT write to them on a cache sync. Sounds counterintuitive, but marking
+ * a reg as "precious" or "volatile" is the only way to avoid this, and that
+ * works only with readable regs.
+ */
+static const struct regmap_range mc33978_readable_range[] = {
+ regmap_reg_range(MC33978_REG_CHECK, MC33978_REG_WET_SG1),
+ regmap_reg_range(MC33978_REG_CWET_SP, MC33978_REG_WDEB_SG),
+ regmap_reg_range(MC33978_REG_AMUX_CTRL, MC33978_REG_RESET),
+};
+
+static const struct regmap_access_table mc33978_readable_table = {
+ .yes_ranges = mc33978_readable_range,
+ .n_yes_ranges = ARRAY_SIZE(mc33978_readable_range),
+};
+
+static const struct regmap_range mc33978_writable_range[] = {
+ regmap_reg_range(MC33978_REG_CONFIG, MC33978_REG_WET_SG1),
+ regmap_reg_range(MC33978_REG_CWET_SP, MC33978_REG_AMUX_CTRL),
+ regmap_reg_range(MC33978_REG_IRQ, MC33978_REG_RESET),
+};
+
+static const struct regmap_access_table mc33978_writable_table = {
+ .yes_ranges = mc33978_writable_range,
+ .n_yes_ranges = ARRAY_SIZE(mc33978_writable_range),
+};
+
+static const struct regmap_config mc33978_regmap_config = {
+ .name = MC33978_DRV_NAME,
+ .reg_bits = 8,
+ .val_bits = 24,
+ .reg_stride = 2,
+ .write_flag_mask = MC33978_CMD_BYTE_WRITE,
+ .reg_format_endian = REGMAP_ENDIAN_BIG,
+ .val_format_endian = REGMAP_ENDIAN_BIG,
+ .use_single_read = true,
+ .use_single_write = true,
+ .volatile_table = &mc33978_volatile_table,
+ .precious_table = &mc33978_precious_table,
+ .rd_table = &mc33978_readable_table,
+ .wr_table = &mc33978_writable_table,
+ .cache_type = REGCACHE_MAPLE,
+ .max_register = MC33978_REG_RESET,
+};
+
+static int mc33978_power_on(struct mc33978_mfd_priv *mc)
+{
+ struct device *dev = &mc->spi->dev;
+ int ret;
+
+ ret = regulator_enable(mc->vddq);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to enable VDDQ supply\n");
+
+ ret = regulator_enable(mc->vbatp);
+ if (ret) {
+ regulator_disable(mc->vddq);
+ return dev_err_probe(dev, ret, "Failed to enable VBATP supply\n");
+ }
+
+ return 0;
+}
+
+static void mc33978_power_off(void *data)
+{
+ struct mc33978_mfd_priv *mc = data;
+
+ regulator_disable(mc->vbatp);
+ regulator_disable(mc->vddq);
+}
+
+/**
+ * mc33978_check_device() - Verify SPI communication with device
+ * @mc: Device context
+ *
+ * Reads the CHECK register which should return a fixed signature (0x123456).
+ * This verifies that SPI communication is working correctly.
+ *
+ * Return: 0 on success, -ENODEV if signature doesn't match
+ */
+static int mc33978_check_device(struct mc33978_mfd_priv *mc)
+{
+ struct device *dev = &mc->spi->dev;
+ unsigned int check;
+ int ret;
+
+ ret = regmap_read(mc->map, MC33978_REG_CHECK, &check);
+ if (ret)
+ return ret;
+
+ if (check != MC33978_CHECK_SIGNATURE)
+ return dev_err_probe(dev, -ENODEV,
+ "SPI check failed. Expected: 0x%06x, got: 0x%06x\n",
+ MC33978_CHECK_SIGNATURE, check);
+
+ return 0;
+}
+
+static const struct resource mc33978_hwmon_resources[] = {
+ DEFINE_RES_IRQ(MC33978_HWIRQ_FAULT),
+};
+
+static const struct mfd_cell mc33978_cells[] = {
+ { .name = "mc33978-pinctrl", .of_compatible = "nxp,mc33978-pinctrl" },
+ {
+ .name = "mc33978-hwmon",
+ .resources = mc33978_hwmon_resources,
+ .num_resources = ARRAY_SIZE(mc33978_hwmon_resources),
+ },
+ { .name = "mc33978-mux" },
+};
+
+static const struct mfd_cell mc34978_cells[] = {
+ { .name = "mc34978-pinctrl", .of_compatible = "nxp,mc34978-pinctrl" },
+ {
+ .name = "mc34978-hwmon",
+ .resources = mc33978_hwmon_resources,
+ .num_resources = ARRAY_SIZE(mc33978_hwmon_resources),
+ },
+ { .name = "mc34978-mux" },
+};
+
+static const struct mc33978_data mc33978_match_data = {
+ .cells = mc33978_cells,
+ .num_cells = ARRAY_SIZE(mc33978_cells),
+};
+
+static const struct mc33978_data mc34978_match_data = {
+ .cells = mc34978_cells,
+ .num_cells = ARRAY_SIZE(mc34978_cells),
+};
+
+static int mc33978_probe(struct spi_device *spi)
+{
+ const struct mc33978_data *match_data;
+ struct device *dev = &spi->dev;
+ struct mc33978_mfd_priv *mc;
+ int ret;
+
+ match_data = spi_get_device_match_data(spi);
+ if (!match_data)
+ return dev_err_probe(dev, -ENODEV, "No device match data found\n");
+
+ mc = devm_kzalloc(dev, sizeof(*mc), GFP_KERNEL);
+ if (!mc)
+ return -ENOMEM;
+
+ mc->spi = spi;
+ spi_set_drvdata(spi, mc);
+
+ mc->vddq = devm_regulator_get(dev, "vddq");
+ if (IS_ERR(mc->vddq))
+ return dev_err_probe(dev, PTR_ERR(mc->vddq),
+ "Failed to get VDDQ regulator\n");
+
+ mc->vbatp = devm_regulator_get(dev, "vbatp");
+ if (IS_ERR(mc->vbatp))
+ return dev_err_probe(dev, PTR_ERR(mc->vbatp),
+ "Failed to get VBATP regulator\n");
+
+ ret = mc33978_power_on(mc);
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(dev, mc33978_power_off, mc);
+ if (ret)
+ return ret;
+
+ mutex_init(&mc->event_lock);
+ INIT_WORK(&mc->event_work, mc33978_event_work);
+
+ atomic_set(&mc->harvested_flags, 0);
+ atomic_set(&mc->is_handling, 0);
+
+ mc33978_prepare_messages(mc);
+
+ mc->map = devm_regmap_init(dev, &mc33978_regmap_bus, mc,
+ &mc33978_regmap_config);
+ if (IS_ERR(mc->map))
+ return dev_err_probe(dev, PTR_ERR(mc->map), "can't init regmap\n");
+
+ ret = mc33978_check_device(mc);
+ if (ret)
+ return dev_err_probe(dev, ret, "Can't use SPI bus\n");
+
+ ret = regmap_write(mc->map, MC33978_REG_IE_SP, 0);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(mc->map, MC33978_REG_IE_SG, 0);
+ if (ret)
+ return ret;
+
+ ret = mc33978_irq_init(mc);
+ if (ret)
+ return ret;
+
+ ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
+ match_data->cells, match_data->num_cells,
+ NULL, 0, mc->domain);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to add MFD child devices\n");
+
+ return 0;
+}
+
+static const struct of_device_id mc33978_of_match[] = {
+ { .compatible = "nxp,mc33978", .data = &mc33978_match_data },
+ { .compatible = "nxp,mc34978", .data = &mc34978_match_data },
+ { }
+};
+MODULE_DEVICE_TABLE(of, mc33978_of_match);
+
+static const struct spi_device_id mc33978_spi_id[] = {
+ { "mc33978", (kernel_ulong_t)&mc33978_match_data },
+ { "mc34978", (kernel_ulong_t)&mc34978_match_data },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, mc33978_spi_id);
+
+static struct spi_driver mc33978_driver = {
+ .driver = {
+ .name = MC33978_DRV_NAME,
+ .of_match_table = mc33978_of_match,
+ },
+ .probe = mc33978_probe,
+ .id_table = mc33978_spi_id,
+};
+module_spi_driver(mc33978_driver);
+
+MODULE_AUTHOR("David Jander <david@protonic.nl>");
+MODULE_DESCRIPTION("NXP MC33978/MC34978 MFD core driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/mc33978.h b/include/linux/mfd/mc33978.h
new file mode 100644
index 000000000000..2d21657581aa
--- /dev/null
+++ b/include/linux/mfd/mc33978.h
@@ -0,0 +1,86 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2024 David Jander <david@protonic.nl>, Protonic Holland
+ * Copyright (C) 2026 Oleksij Rempel <kernel@pengutronix.de>, Pengutronix
+ *
+ * MC34978/MC33978 Multiple Switch Detection Interface - Shared Definitions
+ */
+
+#ifndef _LINUX_MFD_MC34978_H
+#define _LINUX_MFD_MC34978_H
+
+#include <linux/bits.h>
+
+/* Register Map - All addresses are base command bytes (R/W bit = 0) */
+#define MC33978_REG_CHECK 0x00 /* SPI communication check */
+#define MC33978_REG_CONFIG 0x02 /* Device configuration */
+#define MC33978_REG_TRI_SP 0x04 /* Tri-state enable SP */
+#define MC33978_REG_TRI_SG 0x06 /* Tri-state enable SG */
+#define MC33978_REG_WET_SP 0x08 /* Wetting current level SP */
+#define MC33978_REG_WET_SG0 0x0a /* Wetting current level SG0 (SG7-SG0) */
+#define MC33978_REG_WET_SG1 0x0c /* Wetting current level SG1 (SG13-SG8) */
+#define MC33978_REG_CWET_SP 0x16 /* Continuous wetting current SP */
+#define MC33978_REG_CWET_SG 0x18 /* Continuous wetting current SG */
+#define MC33978_REG_IE_SP 0x1a /* Interrupt enable SP */
+#define MC33978_REG_IE_SG 0x1c /* Interrupt enable SG */
+#define MC33978_REG_LPM_CONFIG 0x1e /* Low-power mode configuration */
+#define MC33978_REG_WAKE_SP 0x20 /* Wake-up enable SP */
+#define MC33978_REG_WAKE_SG 0x22 /* Wake-up enable SG */
+#define MC33978_REG_COMP_SP 0x24 /* Comparator only mode SP */
+#define MC33978_REG_COMP_SG 0x26 /* Comparator only mode SG */
+#define MC33978_REG_LPM_VT_SP 0x28 /* LPM voltage threshold SP */
+#define MC33978_REG_LPM_VT_SG 0x2a /* LPM voltage threshold SG */
+#define MC33978_REG_IP_SP 0x2c /* Polling current SP */
+#define MC33978_REG_IP_SG 0x2e /* Polling current SG */
+#define MC33978_REG_SPOLL_SP 0x30 /* Slow polling SP */
+#define MC33978_REG_SPOLL_SG 0x32 /* Slow polling SG */
+#define MC33978_REG_WDEB_SP 0x34 /* Wake-up debounce SP */
+#define MC33978_REG_WDEB_SG 0x36 /* Wake-up debounce SG */
+#define MC33978_REG_ENTER_LPM 0x38 /* Enter low-power mode (write-only) */
+#define MC33978_REG_AMUX_CTRL 0x3a /* AMUX control */
+#define MC33978_REG_READ_IN 0x3e /* Read switch status (READ_SW in datasheet) */
+#define MC33978_REG_FAULT 0x42 /* Fault status register */
+#define MC33978_REG_IRQ 0x46 /* Interrupt request (write-only) */
+#define MC33978_REG_RESET 0x48 /* Reset (write-only) */
+
+/*
+ * FAULT Register (0x42) bit definitions
+ * Reading this register clears most fault flags except persistent conditions
+ */
+#define MC33978_FAULT_SPI_ERROR BIT(10) /* SPI communication error */
+#define MC33978_FAULT_HASH BIT(9) /* SPI register hash mismatch */
+#define MC33978_FAULT_UV BIT(7) /* VBATP undervoltage */
+#define MC33978_FAULT_OV BIT(6) /* VBATP overvoltage */
+#define MC33978_FAULT_TEMP_WARN BIT(5) /* Temperature warning threshold */
+#define MC33978_FAULT_OT BIT(4) /* Over-temperature */
+#define MC33978_FAULT_INTB_WAKE BIT(3) /* Woken by INT_B pin */
+#define MC33978_FAULT_WAKEB_WAKE BIT(2) /* Woken by WAKE_B pin */
+#define MC33978_FAULT_SPI_WAKE BIT(1) /* Woken by SPI message */
+#define MC33978_FAULT_POR BIT(0) /* Power-on reset occurred */
+
+/* Critical faults that need immediate attention */
+#define MC33978_FAULT_CRITICAL (MC33978_FAULT_UV | \
+ MC33978_FAULT_OV | \
+ MC33978_FAULT_OT)
+
+#define MC33978_NUM_PINS 22
+
+/*
+ * Virtual IRQ number for fault handling.
+ * Using hwirq 22 (beyond the 22 pin IRQs 0-21).
+ */
+#define MC33978_HWIRQ_FAULT 22
+
+/*
+ * AMUX channel definitions
+ * The AMUX can route one of 24 signals to the external AMUX pin
+ */
+#define MC33978_AMUX_CH_SG0 0 /* Switch-to-Ground inputs 0-13 */
+#define MC33978_AMUX_CH_SG13 13
+#define MC33978_AMUX_CH_SP0 14 /* Programmable switch inputs 0-7 */
+#define MC33978_AMUX_CH_SP7 21
+#define MC33978_AMUX_CH_TEMP 22 /* Internal temperature diode */
+#define MC33978_AMUX_CH_VBATP 23 /* Battery voltage sense */
+#define MC33978_NUM_AMUX_CH 24 /* Total number of AMUX channels */
+
+#endif /* _LINUX_MFD_MC34978_H */
--
2.47.3
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 3/6] pinctrl: core: Make pin group callbacks optional
2026-03-03 13:39 [PATCH v2 0/8] mfd: Add support for NXP MC33978/MC34978 MSDI Oleksij Rempel
2026-03-03 13:39 ` [PATCH v2 1/6] dt-bindings: mfd: add " Oleksij Rempel
2026-03-03 13:39 ` [PATCH v2 2/6] mfd: add NXP MC33978/MC34978 core driver Oleksij Rempel
@ 2026-03-03 13:39 ` Oleksij Rempel
2026-03-03 13:39 ` [PATCH v2 4/6] pinctrl: add NXP MC33978/MC34978 pinctrl driver Oleksij Rempel
` (2 subsequent siblings)
5 siblings, 0 replies; 25+ messages in thread
From: Oleksij Rempel @ 2026-03-03 13:39 UTC (permalink / raw)
To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lee Jones, Peter Rosin, Linus Walleij
Cc: Oleksij Rempel, kernel, linux-kernel, devicetree, linux-hwmon,
linux-gpio, David Jander
Currently, the pinctrl core strictly requires all drivers to implement
.get_groups_count and .get_group_name callbacks in their pinctrl_ops.
However, for simple pinctrl drivers that act purely as GPIO controllers
and pin-specific configuration proxies (without any concept of muxing or
pin groups), this strict requirement forces the implementation of dummy
callbacks just to satisfy pinctrl_check_ops().
Relax this requirement by making the group callbacks optional. Update
the core and debugfs (pinconf) functions to check for the existence of
these callbacks before invoking them, allowing simple drivers to omit
group boilerplate entirely.
Suggested-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
drivers/pinctrl/core.c | 25 ++++++++++++++++++++-----
drivers/pinctrl/pinconf.c | 18 +++++++++++++-----
2 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index b5e97689589f..920e025622d6 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -621,8 +621,13 @@ static int pinctrl_generic_group_name_to_selector(struct pinctrl_dev *pctldev,
const char *function)
{
const struct pinctrl_ops *ops = pctldev->desc->pctlops;
- int ngroups = ops->get_groups_count(pctldev);
int selector = 0;
+ int ngroups;
+
+ if (!ops->get_groups_count || !ops->get_group_name)
+ return -EINVAL;
+
+ ngroups = ops->get_groups_count(pctldev);
/* See if this pctldev has this group */
while (selector < ngroups) {
@@ -737,8 +742,15 @@ int pinctrl_get_group_selector(struct pinctrl_dev *pctldev,
const char *pin_group)
{
const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
- unsigned int ngroups = pctlops->get_groups_count(pctldev);
unsigned int group_selector = 0;
+ unsigned int ngroups;
+
+ if (!pctlops->get_groups_count || !pctlops->get_group_name) {
+ dev_err(pctldev->dev, "does not support pin groups\n");
+ return -EINVAL;
+ }
+
+ ngroups = pctlops->get_groups_count(pctldev);
while (group_selector < ngroups) {
const char *gname = pctlops->get_group_name(pctldev,
@@ -1769,6 +1781,11 @@ static int pinctrl_groups_show(struct seq_file *s, void *what)
mutex_lock(&pctldev->mutex);
+ if (!ops->get_groups_count || !ops->get_group_name) {
+ mutex_unlock(&pctldev->mutex);
+ return 0;
+ }
+
ngroups = ops->get_groups_count(pctldev);
seq_puts(s, "registered pin groups:\n");
@@ -2050,9 +2067,7 @@ static int pinctrl_check_ops(struct pinctrl_dev *pctldev)
{
const struct pinctrl_ops *ops = pctldev->desc->pctlops;
- if (!ops ||
- !ops->get_groups_count ||
- !ops->get_group_name)
+ if (!ops)
return -EINVAL;
return 0;
diff --git a/drivers/pinctrl/pinconf.c b/drivers/pinctrl/pinconf.c
index dca963633b5d..feab87e8530d 100644
--- a/drivers/pinctrl/pinconf.c
+++ b/drivers/pinctrl/pinconf.c
@@ -273,10 +273,13 @@ void pinconf_show_setting(struct seq_file *s,
setting->data.configs.group_or_pin);
break;
case PIN_MAP_TYPE_CONFIGS_GROUP:
- seq_printf(s, "group %s (%d)",
- pctlops->get_group_name(pctldev,
- setting->data.configs.group_or_pin),
- setting->data.configs.group_or_pin);
+ if (pctlops->get_group_name)
+ seq_printf(s, "group %s (%d)",
+ pctlops->get_group_name(pctldev,
+ setting->data.configs.group_or_pin),
+ setting->data.configs.group_or_pin);
+ else
+ seq_printf(s, "group (%d)", setting->data.configs.group_or_pin);
break;
default:
break;
@@ -348,8 +351,13 @@ static int pinconf_groups_show(struct seq_file *s, void *what)
{
struct pinctrl_dev *pctldev = s->private;
const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
- unsigned int ngroups = pctlops->get_groups_count(pctldev);
unsigned int selector = 0;
+ unsigned int ngroups;
+
+ if (!pctlops->get_groups_count || !pctlops->get_group_name)
+ return 0;
+
+ ngroups = pctlops->get_groups_count(pctldev);
seq_puts(s, "Pin config settings per pin group\n");
seq_puts(s, "Format: group (name): configs\n");
--
2.47.3
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 4/6] pinctrl: add NXP MC33978/MC34978 pinctrl driver
2026-03-03 13:39 [PATCH v2 0/8] mfd: Add support for NXP MC33978/MC34978 MSDI Oleksij Rempel
` (2 preceding siblings ...)
2026-03-03 13:39 ` [PATCH v2 3/6] pinctrl: core: Make pin group callbacks optional Oleksij Rempel
@ 2026-03-03 13:39 ` Oleksij Rempel
2026-03-05 13:41 ` Linus Walleij
2026-03-03 13:39 ` [PATCH v2 5/6] hwmon: add NXP MC33978/MC34978 driver Oleksij Rempel
2026-03-03 13:39 ` [PATCH v2 6/6] mux: add NXP MC33978/MC34978 AMUX driver Oleksij Rempel
5 siblings, 1 reply; 25+ messages in thread
From: Oleksij Rempel @ 2026-03-03 13:39 UTC (permalink / raw)
To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lee Jones, Peter Rosin, Linus Walleij
Cc: David Jander, Oleksij Rempel, kernel, linux-kernel, devicetree,
linux-hwmon, linux-gpio
From: David Jander <david@protonic.nl>
Add pin control and GPIO driver for the NXP MC33978/MC34978 Multiple
Switch Detection Interface (MSDI) devices.
This driver exposes the 22 mechanical switch detection inputs (14
Switch-to-Ground, 8 Programmable) as standard GPIOs.
Key features implemented:
- GPIO read/write: Translates physical switch states (open/closed)
to logical GPIO levels based on the configured switch topology
(Switch-to-Ground vs. Switch-to-Battery).
- Emulated Output: Allows setting pins "high" or "low" by manipulating
the tri-state registers and hardware pull topologies.
- Interrupt routing: Proxies GPIO interrupt requests to the irq_domain
managed by the parent MFD core driver.
Signed-off-by: David Jander <david@protonic.nl>
Co-developed-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v2:
- Translate all remaining German comments to English.
- Remove unnecessary #ifdef CONFIG_OF wrappers around dt_node_to_map.
- Add detailed comments to mc33978_get() and mc33978_get_multiple() explaining
the hardware comparator logic (1 = closed, 0 = open) and justifying the
bitwise inversion required to report actual physical voltage levels.
- Add comments to the .set() and .set_config() callbacks explaining why
gpiolib's standard open-drain emulation (switching to input mode) fails on
this hardware due to active wetting currents, and why tri-state isolation is
mandatory.
- Add a comment to mc33978_gpio_to_irq() explaining why it must act as a
proxy to the parent MFD's irq_domain (shared physical INT_B line with hwmon).
- Drop dummy pin group callbacks (get_groups_count, etc.). This relies on a
preparatory patch in this series making these callbacks optional in the core.
- Fix debugfs 'pinconf-pins' read errors by correctly returning -ENOTSUPP
instead of -EOPNOTSUPP for unsupported generic configurations.
- Fix empty 'gpio-ranges' and missing debugfs labels by explicitly calling
gpiochip_add_pin_range() during probe.
- Eliminate "magic" bitwise math in the wetting current configuration by
introducing a static lookup array (mc33978_wet_mA).
- Resolve checkpatch.pl strict warnings regarding macro argument reuse by
converting MC33978_SPSG, MC33978_PINSHIFT, MC33978_WREG, and MC33978_WSHIFT
to static inline functions.
- Remove artifacts from previous interrupt handling implementations.
- Address minor formatting and whitespace nits.
---
drivers/pinctrl/Kconfig | 14 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinctrl-mc33978.c | 733 ++++++++++++++++++++++++++++++
3 files changed, 748 insertions(+)
create mode 100644 drivers/pinctrl/pinctrl-mc33978.c
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index afecd9407f53..c315656c0fe5 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -388,6 +388,20 @@ config PINCTRL_MAX77620
function in alternate mode. This driver also configure push-pull,
open drain, FPS slots etc.
+config PINCTRL_MC33978
+ tristate "MC33978/MC34978 industrial input controller support"
+ depends on MFD_MC33978
+ select GPIOLIB
+ select GENERIC_PINCONF
+ help
+ Say Y here to enable support for NXP MC33978/MC34978 Multiple
+ Switch Detection Interface (MSDI) devices. This driver provides
+ pinctrl and GPIO interfaces for the 22 mechanical switch inputs
+ (14 Switch-to-Ground, 8 Programmable).
+
+ It allows reading switch states, configuring hardware pull
+ topologies, and handling interrupts for state changes.
+
config PINCTRL_MCP23S08_I2C
tristate
select REGMAP_I2C
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index f7d5d5f76d0c..afb58fb5a197 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_PINCTRL_XWAY) += pinctrl-xway.o
obj-$(CONFIG_PINCTRL_LPC18XX) += pinctrl-lpc18xx.o
obj-$(CONFIG_PINCTRL_MAX7360) += pinctrl-max7360.o
obj-$(CONFIG_PINCTRL_MAX77620) += pinctrl-max77620.o
+obj-$(CONFIG_PINCTRL_MC33978) += pinctrl-mc33978.o
obj-$(CONFIG_PINCTRL_MCP23S08_I2C) += pinctrl-mcp23s08_i2c.o
obj-$(CONFIG_PINCTRL_MCP23S08_SPI) += pinctrl-mcp23s08_spi.o
obj-$(CONFIG_PINCTRL_MCP23S08) += pinctrl-mcp23s08.o
diff --git a/drivers/pinctrl/pinctrl-mc33978.c b/drivers/pinctrl/pinctrl-mc33978.c
new file mode 100644
index 000000000000..69128b9012bd
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-mc33978.c
@@ -0,0 +1,733 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024 David Jander <david@protonic.nl>, Protonic Holland
+ * Copyright (C) 2026 Oleksij Rempel <kernel@pengutronix.de>, Pengutronix
+ *
+ * MC33978/MC34978 Multiple Switch Detection Interface - Pinctrl/GPIO Driver
+ *
+ * Provides GPIO and pinctrl interfaces for the 22 switch detection inputs.
+ * Handles digital input reading and wetting current configuration. Analog AMUX
+ * functionality is handled by a separate mux driver.
+ *
+ * GPIO Mapping:
+ * - GPIO 0-13: SG0-SG13 (Switch-to-Ground inputs)
+ * - GPIO 14-21: SP0-SP7 (Programmable: Switch-to-Ground or Switch-to-Battery)
+ * This is dictated by the READ_IN register where bits [21:14] = SP[7:0]
+ * and bits [13:0] = SG[13:0].
+ *
+ * Register Organization:
+ * Configuration registers are generally paired. The _SP register at offset N
+ * controls SP0-SP7, and the _SG register at offset N+2 controls SG0-SG13.
+ *
+ * Wetting Currents vs. Pull Resistors:
+ * The hardware physically lacks traditional passive pull-up or pull-down
+ * resistors. Instead, it uses active, controllable current regulators
+ * (wetting currents) to detect switch states and clean mechanical contacts.
+ * - Because these are active current sources, specifying an ohmic value for
+ * pull-up/down biases is physically invalid. The driver ignores ohm arguments.
+ * - 8 selectable current values: 2, 6, 8, 10, 12, 14, 16, 20 mA.
+ * - Exposed via the pinconf PIN_CONFIG_DRIVE_STRENGTH parameter (in mA).
+ *
+ * Emulated Outputs:
+ * The hardware lacks traditional push-pull output drivers; it is strictly an
+ * input device. "Outputs" are simulated by toggling the wetting currents and
+ * physically isolating the pins via hardware tri-state registers. Consequently,
+ * consumers MUST flag outputs with GPIO_OPEN_DRAIN or GPIO_OPEN_SOURCE in
+ * the Device Tree.
+ *
+ * Input Detection Mechanics:
+ * This input mechanism relies on the active current regulators rather than
+ * passive hard resistors. For a Switch-to-Ground (SG) pin, the chip sources
+ * a constant current. When the switch is open, the pin voltage floats up to
+ * the battery voltage. When the switch closes, it creates a path to ground;
+ * because the current is strictly regulated, the pin voltage drops sharply
+ * below the internal 4.0V comparator threshold.
+ * * The hardware evaluates this and reports an abstract "contact status"
+ * (1 = closed, 0 = open). For SG pins, a closed switch (~0V) reports as '1'.
+ * To align with gpiolib expectations where ~0V equals a physical logical '0',
+ * this driver explicitly inverts the hardware status for all SG-configured
+ * pins before reporting them.
+ *
+ * Interrupts:
+ * The physical INT_B line and threaded IRQ domain are managed centrally by
+ * the parent MFD core. This driver simply proxies .to_irq() to the parent.
+ *
+ * Written by David Jander <david@protonic.nl>
+ *
+ * Datasheet:
+ * https://www.nxp.com/docs/en/data-sheet/MC33978.pdf
+ */
+
+#include <linux/device.h>
+#include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/seq_file.h>
+
+#include <linux/mfd/mc33978.h>
+
+#define MC33978_NGPIO 22
+
+/*
+ * Input numbering is dictated by bit-order of the input register:
+ * Inputs 0-13 -> SG0-SG13
+ * Inputs 14-21 -> SP0-SP7
+ */
+#define MC33978_NUM_SG 14
+#define MC33978_SP_MASK GENMASK(MC33978_NGPIO - 1, MC33978_NUM_SG)
+#define MC33978_SG_MASK GENMASK(MC33978_NUM_SG - 1, 0)
+#define MC33978_SG_SHIFT 0
+#define MC33978_SP_SHIFT MC33978_NUM_SG
+
+#define MC33978_TRISTATE 0
+#define MC33978_PU 1
+#define MC33978_PD 2
+
+struct mc33978_pinctrl {
+ struct device *dev;
+ struct regmap *regmap;
+ int irq;
+
+ struct irq_domain *domain;
+
+ struct gpio_chip chip;
+ struct pinctrl_dev *pctldev;
+ struct pinctrl_desc pinctrl_desc;
+
+ /*
+ * Protects multi-register hardware sequences in .set() and atomic
+ * READ_IN + CONFIG reads in .get()
+ */
+ struct mutex lock;
+};
+
+static const struct pinctrl_pin_desc mc33978_pins[] = {
+ PINCTRL_PIN(0, "sg0"),
+ PINCTRL_PIN(1, "sg1"),
+ PINCTRL_PIN(2, "sg2"),
+ PINCTRL_PIN(3, "sg3"),
+ PINCTRL_PIN(4, "sg4"),
+ PINCTRL_PIN(5, "sg5"),
+ PINCTRL_PIN(6, "sg6"),
+ PINCTRL_PIN(7, "sg7"),
+ PINCTRL_PIN(8, "sg8"),
+ PINCTRL_PIN(9, "sg9"),
+ PINCTRL_PIN(10, "sg10"),
+ PINCTRL_PIN(11, "sg11"),
+ PINCTRL_PIN(12, "sg12"),
+ PINCTRL_PIN(13, "sg13"),
+ PINCTRL_PIN(14, "sp0"),
+ PINCTRL_PIN(15, "sp1"),
+ PINCTRL_PIN(16, "sp2"),
+ PINCTRL_PIN(17, "sp3"),
+ PINCTRL_PIN(18, "sp4"),
+ PINCTRL_PIN(19, "sp5"),
+ PINCTRL_PIN(20, "sp6"),
+ PINCTRL_PIN(21, "sp7"),
+};
+
+static inline bool mc33978_is_sp(unsigned int pin)
+{
+ return pin >= MC33978_NUM_SG;
+}
+
+/* Choose register offset for _SG/_SP registers. reg is always the _SP addr. */
+static inline u8 mc33978_spsg(u8 reg, unsigned int pin)
+{
+ return mc33978_is_sp(pin) ? reg : reg + 2;
+}
+
+/* Get the bit index into the corresponding register */
+static inline unsigned int mc33978_pinshift(unsigned int pin)
+{
+ return mc33978_is_sp(pin) ? pin - MC33978_NUM_SG : pin;
+}
+
+#define MC33978_PINMASK(pin) BIT(mc33978_pinshift(pin))
+
+/*
+ * Wetting current registers: 3 in total, each pin uses a 3-bit field,
+ * 8 pins per register, except for the last one.
+ */
+static inline u8 mc33978_wreg(u8 reg, unsigned int pin)
+{
+ return reg + (mc33978_is_sp(pin) ? 0 : 2 + 2 * (pin / 8));
+}
+
+static inline unsigned int mc33978_wshift(unsigned int pin)
+{
+ return mc33978_is_sp(pin) ? 3 * (pin - MC33978_NUM_SG) : 3 * (pin % 8);
+}
+
+#define MC33978_WMASK(pin) (7 << mc33978_wshift(pin))
+
+static int mc33978_read(struct mc33978_pinctrl *mpc, u8 reg, u32 *val)
+{
+ int ret;
+
+ ret = regmap_read(mpc->regmap, reg, val);
+ if (ret)
+ dev_err_ratelimited(mpc->dev, "Regmap read error %d at reg: %02x.\n",
+ ret, reg);
+ return ret;
+}
+
+static int mc33978_update_bits(struct mc33978_pinctrl *mpc, u8 reg, u32 mask, u32 val)
+{
+ int ret;
+
+ ret = regmap_update_bits(mpc->regmap, reg, mask, val);
+ if (ret)
+ dev_err_ratelimited(mpc->dev, "Regmap update bits error %d at reg: %02x.\n",
+ ret, reg);
+ return ret;
+}
+
+static const struct pinctrl_ops mc33978_pinctrl_ops = {
+ .dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
+ .dt_free_map = pinconf_generic_dt_free_map,
+};
+
+static int mc33978_get_pull(struct mc33978_pinctrl *mpc, unsigned int pin, int *val)
+{
+ unsigned int data;
+ int ret;
+
+ ret = mc33978_read(mpc, mc33978_spsg(MC33978_REG_TRI_SP, pin), &data);
+ if (ret < 0)
+ return ret;
+
+ /* Is the pin tri-stated? */
+ if (data & MC33978_PINMASK(pin)) {
+ *val = MC33978_TRISTATE;
+ return 0;
+ }
+
+ /* Pins 0..13 only support pull-up */
+ if (!mc33978_is_sp(pin)) {
+ *val = MC33978_PU;
+ return 0;
+ }
+
+ /* Check pin pull direction for pins 14..21 */
+ ret = mc33978_read(mpc, MC33978_REG_CONFIG, &data);
+ if (ret < 0)
+ return ret;
+ if (data & MC33978_PINMASK(pin))
+ *val = MC33978_PD;
+ else
+ *val = MC33978_PU;
+ return 0;
+}
+
+static int mc33978_set_pull(struct mc33978_pinctrl *mpc, unsigned int pin, int val)
+{
+ unsigned int mask = MC33978_PINMASK(pin);
+ int ret;
+
+ /* SG pins physically lack pull-downs current sources */
+ if (val == MC33978_PD && !mc33978_is_sp(pin))
+ return -EINVAL;
+
+ /* Configure direction (Exclusively for SP pins) */
+ if (mc33978_is_sp(pin) && val != MC33978_TRISTATE) {
+ ret = mc33978_update_bits(mpc, MC33978_REG_CONFIG, mask,
+ (val == MC33978_PD) ? mask : 0);
+ if (ret)
+ return ret;
+ }
+
+ /* Enable current source or set to tri-state */
+ ret = mc33978_update_bits(mpc, mc33978_spsg(MC33978_REG_TRI_SP, pin),
+ mask,
+ (val == MC33978_TRISTATE) ? mask : 0);
+ return ret;
+}
+
+static const unsigned int mc33978_wet_mA[] = { 2, 6, 8, 10, 12, 14, 16, 20 };
+
+static int mc33978_set_ds(struct mc33978_pinctrl *mpc, unsigned int pin,
+ unsigned int val)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(mc33978_wet_mA); i++) {
+ if (val == mc33978_wet_mA[i]) {
+ return mc33978_update_bits(mpc,
+ mc33978_wreg(MC33978_REG_WET_SP, pin),
+ MC33978_WMASK(pin),
+ i << mc33978_wshift(pin));
+ }
+ }
+
+ return -EINVAL;
+}
+
+static int mc33978_get_ds(struct mc33978_pinctrl *mpc, unsigned int pin,
+ unsigned int *val)
+{
+ unsigned int data;
+ int ret;
+
+ ret = mc33978_read(mpc, mc33978_wreg(MC33978_REG_WET_SP, pin), &data);
+ if (ret)
+ return ret;
+
+ data &= MC33978_WMASK(pin);
+ data >>= mc33978_wshift(pin);
+
+ if (data >= ARRAY_SIZE(mc33978_wet_mA))
+ return -EINVAL;
+
+ *val = mc33978_wet_mA[data];
+
+ return 0;
+}
+
+static int mc33978_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin,
+ unsigned long *config)
+{
+ struct mc33978_pinctrl *mpc = pinctrl_dev_get_drvdata(pctldev);
+ enum pin_config_param param = pinconf_to_config_param(*config);
+ unsigned int data, status;
+ int ret;
+
+ switch (param) {
+ case PIN_CONFIG_BIAS_PULL_UP:
+ ret = mc33978_get_pull(mpc, pin, &data);
+ if (ret)
+ return ret;
+ status = (data == MC33978_PU);
+ break;
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ ret = mc33978_get_pull(mpc, pin, &data);
+ if (ret)
+ return ret;
+ status = (data == MC33978_PD);
+ break;
+ case PIN_CONFIG_BIAS_DISABLE:
+ case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
+ ret = mc33978_get_pull(mpc, pin, &data);
+ if (ret)
+ return ret;
+ status = (data == MC33978_TRISTATE);
+ break;
+ case PIN_CONFIG_DRIVE_STRENGTH:
+ ret = mc33978_get_ds(mpc, pin, &data);
+ if (ret)
+ return ret;
+ *config = pinconf_to_config_packed(param, data);
+ status = 1;
+ break;
+ default:
+ /*
+ * Ignore checkpatch warning: the pinctrl core specifically
+ * expects -ENOTSUPP to silently skip unsupported generic
+ * parameters. Using -EOPNOTSUPP causes debugfs read failures.
+ */
+ return -ENOTSUPP;
+ }
+
+ return status ? 0 : -EINVAL;
+}
+
+/*
+ * Hardware constraint regarding PIN_CONFIG_BIAS_PULL_UP/DOWN:
+ * The MC33978 utilizes active constant current sources (wetting currents)
+ * rather than passive pull-resistors. Since the equivalent ohmic resistance
+ * scales dynamically with the fluctuating board voltage (VBATP), computing
+ * a static ohm value is physically invalid.
+ * The driver intentionally ignores resistance arguments during configuration
+ * and continuously reports 0 ohms to the pinctrl framework.
+ */
+static int mc33978_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
+ unsigned long *configs, unsigned int num_configs)
+{
+ struct mc33978_pinctrl *mpc = pinctrl_dev_get_drvdata(pctldev);
+ enum pin_config_param param;
+ int ret = 0;
+ u32 arg;
+ int i;
+
+ for (i = 0; i < num_configs; i++) {
+ param = pinconf_to_config_param(configs[i]);
+ arg = pinconf_to_config_argument(configs[i]);
+
+ /*
+ * The hardware physically lacks push-pull output drivers.
+ * By explicitly handling OPEN_DRAIN and OPEN_SOURCE here, we
+ * signal to gpiolib that we support these modes "natively".
+ * This crucially prevents gpiolib from falling back to its
+ * software emulation (which sets the pin to input mode to
+ * achieve High-Z). On the MC33978, input mode is NOT High-Z;
+ * it actively drives the line with a wetting current!
+ */
+ switch (param) {
+ case PIN_CONFIG_DRIVE_OPEN_SOURCE:
+ case PIN_CONFIG_BIAS_PULL_UP:
+ ret = mc33978_set_pull(mpc, pin, MC33978_PU);
+ break;
+ case PIN_CONFIG_DRIVE_OPEN_DRAIN:
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ if (!mc33978_is_sp(pin)) {
+ dev_err(mpc->dev, "Pin %u is SG and does not support pull-down\n",
+ pin);
+ return -EINVAL;
+ }
+ ret = mc33978_set_pull(mpc, pin, MC33978_PD);
+ break;
+ case PIN_CONFIG_DRIVE_STRENGTH_UA:
+ arg /= 1000;
+ fallthrough;
+ case PIN_CONFIG_DRIVE_STRENGTH:
+ ret = mc33978_set_ds(mpc, pin, arg);
+ break;
+ default:
+ /*
+ * Required by the pinctrl core to safely fall back or
+ * skip unsupported configs. Do not use -EOPNOTSUPP.
+ */
+ return -ENOTSUPP;
+ }
+
+ if (ret) {
+ dev_err(mpc->dev, "Failed to set config param %04x for pin %u: %d\n",
+ param, pin, ret);
+ return ret;
+ }
+ }
+
+ return ret;
+}
+
+static const struct pinconf_ops mc33978_pinconf_ops = {
+ .pin_config_get = mc33978_pinconf_get,
+ .pin_config_set = mc33978_pinconf_set,
+ .is_generic = true,
+};
+
+static int mc33978_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+ struct mc33978_pinctrl *mpc = gpiochip_get_data(chip);
+
+ /*
+ * This chip only has inputs. We emulate outputs by setting a
+ * wetting current and/or using the tri-state register to turn it on
+ * and off. If a pin was an output and is now tri-stated, we should
+ * disable the tri-state now to make the input work correctly.
+ */
+ mutex_lock(&mpc->lock);
+ mc33978_update_bits(mpc, mc33978_spsg(MC33978_REG_TRI_SP, offset),
+ MC33978_PINMASK(offset), 0);
+ mutex_unlock(&mpc->lock);
+
+ return 0;
+}
+
+static int mc33978_get(struct gpio_chip *chip, unsigned int offset)
+{
+ struct mc33978_pinctrl *mpc = gpiochip_get_data(chip);
+ bool is_switch_to_ground = true;
+ bool is_switch_closed;
+ int status, ret;
+
+ mutex_lock(&mpc->lock);
+
+ /* Read hardware switch status (open or closed) */
+ ret = mc33978_read(mpc, MC33978_REG_READ_IN, &status);
+ if (ret < 0) {
+ mutex_unlock(&mpc->lock);
+ return 0;
+ }
+ is_switch_closed = !!(status & BIT(offset));
+
+ /* Determine current topology for SP pins */
+ if (mc33978_is_sp(offset)) {
+ int config_reg;
+
+ ret = mc33978_read(mpc, MC33978_REG_CONFIG, &config_reg);
+ if (ret == 0) {
+ /* CONFIG: 0 = Switch-to-Ground (PU), 1 = Switch-to-Battery (PD) */
+ if (config_reg & MC33978_PINMASK(offset))
+ is_switch_to_ground = false;
+ }
+ }
+
+ mutex_unlock(&mpc->lock);
+
+ /*
+ * The hardware evaluates pin voltage against a threshold (default 4.0V)
+ * and reports an abstract contact status (1 = closed, 0 = open):
+ *
+ * SG (Switch-to-Ground) topology (pull-up current source):
+ * - Voltage > Threshold: Switch Open (HW reports 0) -> Physical High
+ * - Voltage < Threshold: Switch Closed (HW reports 1) -> Physical Low
+ *
+ * SB (Switch-to-Battery) topology (pull-down current source):
+ * - Voltage > Threshold: Switch Closed (HW reports 1) -> Physical High
+ * - Voltage < Threshold: Switch Open (HW reports 0) -> Physical Low
+ *
+ * We translate this contact status back into physical voltage levels.
+ * This translation is necessary because the hardware's contact status
+ * (e.g., reporting 1 when an SG pin is grounded) contradicts the
+ * typical GPIO expectation where ~0V corresponds to a logical 0.
+ */
+ if (is_switch_to_ground)
+ status = !is_switch_closed;
+ else
+ status = is_switch_closed;
+
+ return status;
+}
+
+static int mc33978_get_multiple(struct gpio_chip *chip,
+ unsigned long *mask, unsigned long *bits)
+{
+ struct mc33978_pinctrl *mpc = gpiochip_get_data(chip);
+ unsigned int status;
+ unsigned int config_reg = 0;
+ unsigned int inv_mask;
+ int ret;
+
+ mutex_lock(&mpc->lock);
+
+ ret = mc33978_read(mpc, MC33978_REG_READ_IN, &status);
+ if (ret)
+ goto out_unlock;
+
+ /* Read CONFIG register only if the requested mask involves SP pins */
+ if (*mask & MC33978_SP_MASK) {
+ ret = mc33978_read(mpc, MC33978_REG_CONFIG, &config_reg);
+ if (ret)
+ goto out_unlock;
+ }
+
+ /*
+ * Create an inversion mask for all pins currently operating in
+ * Switch-to-Ground (SG) topology. As explained in mc33978_get(),
+ * SG pins must have their hardware status bits inverted to
+ * correctly report their physical voltage levels.
+ */
+ inv_mask = MC33978_SG_MASK |
+ (~(config_reg << MC33978_NUM_SG) & MC33978_SP_MASK);
+
+ *bits = (status ^ inv_mask) & *mask;
+
+out_unlock:
+ mutex_unlock(&mpc->lock);
+
+ return ret;
+}
+
+static int mc33978_set(struct gpio_chip *chip, unsigned int offset, int value)
+{
+ struct mc33978_pinctrl *mpc = gpiochip_get_data(chip);
+ int pull;
+ int ret;
+
+ /*
+ * We emulate open-drain/-source outputs by routing or isolating the
+ * active wetting current sources.
+ * To drive the line, we apply the current source.
+ * To turn the line OFF (achieve High-Impedance), we MUST use the
+ * hardware TRI_SP / TRI_SG tri-state registers to physically isolate
+ * it.
+ */
+ if (mc33978_is_sp(offset)) {
+ pull = value ? MC33978_PU : MC33978_PD;
+ value = 1;
+ } else {
+ pull = MC33978_PU;
+ }
+
+ mutex_lock(&mpc->lock);
+
+ /*
+ * Break-before-make sequencing to prevent hardware glitches (spikes).
+ * Since SPI transfers take time, writing the pull and tri-state
+ * registers in the wrong order causes a brief moment where current
+ * flows to the pin before it is masked, causing a visible LED flash.
+ */
+ if (value) {
+ /*
+ * Turn ON: Configure the underlying current source (pull) first,
+ * then route it to the pin by disabling tri-state.
+ */
+ ret = mc33978_set_pull(mpc, offset, pull);
+ if (ret)
+ goto out_unlock;
+
+ ret = mc33978_update_bits(mpc, mc33978_spsg(MC33978_REG_TRI_SP,
+ offset),
+ MC33978_PINMASK(offset), 0);
+ } else {
+ /*
+ * Turn OFF: Isolate the pin first by enabling tri-state,
+ * then safely disable the underlying current source.
+ */
+ ret = mc33978_update_bits(mpc, mc33978_spsg(MC33978_REG_TRI_SP,
+ offset),
+ MC33978_PINMASK(offset),
+ MC33978_PINMASK(offset));
+ }
+
+out_unlock:
+ mutex_unlock(&mpc->lock);
+
+ return ret;
+}
+
+static int mc33978_set_multiple(struct gpio_chip *chip,
+ unsigned long *mask, unsigned long *bits)
+{
+ unsigned int sgmask = (*mask & MC33978_SG_MASK) >> MC33978_SG_SHIFT;
+ unsigned int sgbits = (*bits & MC33978_SG_MASK) >> MC33978_SG_SHIFT;
+ unsigned int spmask = (*mask & MC33978_SP_MASK) >> MC33978_SP_SHIFT;
+ unsigned int spbits = (*bits & MC33978_SP_MASK) >> MC33978_SP_SHIFT;
+ struct mc33978_pinctrl *mpc = gpiochip_get_data(chip);
+
+ mutex_lock(&mpc->lock);
+ if (spmask)
+ mc33978_update_bits(mpc, MC33978_REG_TRI_SP, spmask, ~spbits);
+ if (sgmask)
+ mc33978_update_bits(mpc, MC33978_REG_TRI_SG, sgmask, ~sgbits);
+ mutex_unlock(&mpc->lock);
+
+ return 0;
+}
+
+static int mc33978_direction_output(struct gpio_chip *chip, unsigned int offset,
+ int value)
+{
+ return mc33978_set(chip, offset, value);
+}
+
+static int mc33978_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
+{
+ struct mc33978_pinctrl *mpc = gpiochip_get_data(chip);
+ int virq;
+
+ if (!mpc->domain)
+ return -ENXIO;
+
+ /*
+ * The hardware shares a single physical INT_B line for both GPIO pin
+ * changes and internal hardware faults (hwmon). Therefore, the IRQ
+ * domain and threaded handler are centrally managed by the MFD core.
+ */
+ virq = irq_create_mapping(mpc->domain, offset);
+ if (!virq) {
+ dev_err(mpc->dev, "Failed to map hwirq %u to virq\n", offset);
+ return -ENXIO;
+ }
+
+ return virq;
+}
+
+static void mc33978_init_gpio_chip(struct mc33978_pinctrl *mpc,
+ struct device *dev)
+{
+ mpc->chip.label = dev_name(dev);
+ mpc->chip.direction_input = mc33978_direction_input;
+ mpc->chip.get = mc33978_get;
+ mpc->chip.get_multiple = mc33978_get_multiple;
+ mpc->chip.direction_output = mc33978_direction_output;
+ mpc->chip.set = mc33978_set;
+ mpc->chip.set_multiple = mc33978_set_multiple;
+ mpc->chip.set_config = gpiochip_generic_config;
+
+ mpc->chip.to_irq = mc33978_gpio_to_irq;
+
+ mpc->chip.base = -1;
+ mpc->chip.ngpio = MC33978_NGPIO;
+ mpc->chip.can_sleep = true;
+ mpc->chip.parent = dev;
+ mpc->chip.owner = THIS_MODULE;
+}
+
+static void mc33978_init_pinctrl_desc(struct mc33978_pinctrl *mpc,
+ struct device *dev)
+{
+ mpc->pinctrl_desc.name = dev_name(dev);
+
+ mpc->pinctrl_desc.pctlops = &mc33978_pinctrl_ops;
+ mpc->pinctrl_desc.confops = &mc33978_pinconf_ops;
+ mpc->pinctrl_desc.pins = mc33978_pins;
+ mpc->pinctrl_desc.npins = MC33978_NGPIO;
+ mpc->pinctrl_desc.owner = THIS_MODULE;
+}
+
+static int mc33978_pinctrl_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mc33978_pinctrl *mpc;
+ struct device_node *np = dev->of_node;
+ int ret;
+
+ if (!np)
+ return dev_err_probe(dev, -EINVAL, "Missing device tree node\n");
+
+ mpc = devm_kzalloc(dev, sizeof(*mpc), GFP_KERNEL);
+ if (!mpc)
+ return -ENOMEM;
+
+ mpc->dev = dev;
+
+ mpc->regmap = dev_get_regmap(dev->parent, NULL);
+ if (!mpc->regmap)
+ return dev_err_probe(dev, -ENODEV, "Failed to get parent regmap\n");
+
+ mpc->domain = irq_find_host(dev->parent->of_node);
+ if (!mpc->domain)
+ return dev_err_probe(dev, -ENODEV, "Failed to find parent IRQ domain\n");
+
+ mutex_init(&mpc->lock);
+
+ mc33978_init_gpio_chip(mpc, dev);
+ mc33978_init_pinctrl_desc(mpc, dev);
+
+ mpc->pctldev = devm_pinctrl_register(dev, &mpc->pinctrl_desc, mpc);
+ if (IS_ERR(mpc->pctldev))
+ return dev_err_probe(dev, PTR_ERR(mpc->pctldev),
+ "can't register pinctrl\n");
+
+ ret = devm_gpiochip_add_data(dev, &mpc->chip, mpc);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "can't add GPIO chip\n");
+
+ ret = gpiochip_add_pin_range(&mpc->chip, dev_name(dev), 0, 0,
+ MC33978_NGPIO);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to add pin range\n");
+
+ platform_set_drvdata(pdev, mpc);
+
+ return 0;
+}
+
+static const struct of_device_id mc33978_pinctrl_of_match[] = {
+ { .compatible = "nxp,mc33978-pinctrl" },
+ { .compatible = "nxp,mc34978-pinctrl" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, mc33978_pinctrl_of_match);
+
+static struct platform_driver mc33978_pinctrl_driver = {
+ .driver = {
+ .name = "mc33978-pinctrl",
+ .of_match_table = mc33978_pinctrl_of_match,
+ },
+ .probe = mc33978_pinctrl_probe,
+};
+module_platform_driver(mc33978_pinctrl_driver);
+
+MODULE_AUTHOR("David Jander <david@protonic.nl>");
+MODULE_DESCRIPTION("NXP MC33978/MC33978 pinctrl driver");
+MODULE_LICENSE("GPL");
--
2.47.3
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 5/6] hwmon: add NXP MC33978/MC34978 driver
2026-03-03 13:39 [PATCH v2 0/8] mfd: Add support for NXP MC33978/MC34978 MSDI Oleksij Rempel
` (3 preceding siblings ...)
2026-03-03 13:39 ` [PATCH v2 4/6] pinctrl: add NXP MC33978/MC34978 pinctrl driver Oleksij Rempel
@ 2026-03-03 13:39 ` Oleksij Rempel
2026-03-03 13:39 ` [PATCH v2 6/6] mux: add NXP MC33978/MC34978 AMUX driver Oleksij Rempel
5 siblings, 0 replies; 25+ messages in thread
From: Oleksij Rempel @ 2026-03-03 13:39 UTC (permalink / raw)
To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lee Jones, Peter Rosin, Linus Walleij
Cc: Oleksij Rempel, kernel, linux-kernel, devicetree, linux-hwmon,
linux-gpio, David Jander
Add hardware monitoring support for the NXP MC33978/MC34978 MSDI.
The driver exposes static operating thresholds (thermal, over-voltage,
under-voltage) and reports dynamic hardware fault alarms.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v2:
- Switch from OF match table to platform_device_id
---
drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/mc33978-hwmon.c | 430 ++++++++++++++++++++++++++++++++++
3 files changed, 441 insertions(+)
create mode 100644 drivers/hwmon/mc33978-hwmon.c
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 41c381764c2b..c5d99510cc00 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -700,6 +700,16 @@ config SENSORS_MC13783_ADC
help
Support for the A/D converter on MC13783 and MC13892 PMIC.
+config SENSORS_MC33978
+ tristate "NXP MC33978/MC34978 fault monitoring"
+ depends on MFD_MC33978
+ help
+ If you say yes here you get fault monitoring support for the
+ NXP MC33978/MC34978 Multiple Switch Detection Interface (MSDI).
+
+ This driver can also be built as a module. If so, the module
+ will be called mc33978-hwmon.
+
config SENSORS_MC33XS2410
tristate "MC33XS2410 HWMON support"
depends on PWM_MC33XS2410
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index eade8e3b1bde..e40bc29b9850 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -167,6 +167,7 @@ obj-$(CONFIG_SENSORS_MAX31790) += max31790.o
obj-$(CONFIG_MAX31827) += max31827.o
obj-$(CONFIG_SENSORS_MAX77705) += max77705-hwmon.o
obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o
+obj-$(CONFIG_SENSORS_MC33978) += mc33978-hwmon.o
obj-$(CONFIG_SENSORS_MC33XS2410) += mc33xs2410_hwmon.o
obj-$(CONFIG_SENSORS_MC34VR500) += mc34vr500.o
obj-$(CONFIG_SENSORS_MCP3021) += mcp3021.o
diff --git a/drivers/hwmon/mc33978-hwmon.c b/drivers/hwmon/mc33978-hwmon.c
new file mode 100644
index 000000000000..8740523773f7
--- /dev/null
+++ b/drivers/hwmon/mc33978-hwmon.c
@@ -0,0 +1,430 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Copyright (c) 2026 Pengutronix, Oleksij Rempel <kernel@pengutronix.de>
+/*
+ * MC33978/MC34978 Hardware Monitor Driver
+ *
+ * CRITICAL HARDWARE BEHAVIOR - THERMAL (tLIM):
+ * When the thermal limit (>155°C) is reached, the IC autonomously
+ * reduces the continuous wetting current (CWET) to 2.0 mA to prevent
+ * thermal destruction. This throttling persists until the silicon cools
+ * down below 140°C (15°C hysteresis).
+ *
+ * WARNING FOR PINCTRL/GPIO CONSUMERS:
+ * During an active tLIM fault, the switch state detection becomes
+ * inherently unreliable. A throttled wetting current of 2.0 mA may
+ * be insufficient to break through the oxide layer of mechanical
+ * contacts in the field, leading to false-open GPIO readings.
+ *
+ * VOLTAGE DEGRADATION WARNING (VBATP):
+ * While the hard Undervoltage Lockout (UVLO) asserts strictly at 4.5V,
+ * the silicon operates with degraded parametrics whenever the supply
+ * drops below 6.0V. System designers must be aware that analog routing
+ * (AMUX) and switch detection logic may behave non-deterministically
+ * before the actual UV alarm triggers.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/interrupt.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <linux/mfd/mc33978.h>
+
+/* Thermal Warning threshold (~120C) */
+#define MC33978_TEMP_WARN_MC 120000
+
+/* Thermal Limit / tLIM (>155C) - Hardware enters CWET throttling */
+#define MC33978_TEMP_CRIT_MC 155000
+
+/* Hysteresis for tLIM recovery (Silicon must cool to <140C) */
+#define MC33978_TEMP_HYST_MC 15000
+
+/* VBATP (in0) IC Level thresholds */
+#define MC33978_VBATP_OV_MV 36000 /* Overvoltage limit */
+#define MC33978_VBATP_FUNC_MV 28000 /* Functional/Normal boundary */
+#define MC33978_VBATP_DEGRADED_MV 6000 /* Degraded parametrics start */
+#define MC33978_VBATP_UVLO_MV 4500 /* UV Rising Threshold max */
+
+/* VDDQ (in1) Logic Supply thresholds */
+#define MC33978_VDDQ_MAX_MV 5250 /* Operating Condition max */
+#define MC33978_VDDQ_MIN_MV 3000 /* Operating Condition min */
+#define MC33978_VDDQ_UV_MV 2800 /* UV Falling Threshold max */
+
+enum mc33978_hwmon_in_channels {
+ MC33978_IN_VBATP,
+ MC33978_IN_VDDQ,
+};
+
+struct mc33978_hwmon_priv {
+ struct device *dev;
+ struct device *hwmon_dev;
+ struct regmap *map;
+ int fault_irq;
+ u32 last_faults;
+};
+
+static int mc33978_hwmon_read_fault(struct mc33978_hwmon_priv *priv,
+ u32 *faults)
+{
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(priv->map, MC33978_REG_FAULT, &val);
+ if (ret)
+ return ret;
+
+ *faults = val;
+
+ return 0;
+}
+
+static void mc33978_hwmon_report_faults(struct mc33978_hwmon_priv *priv,
+ u32 new_faults)
+{
+ /*
+ * Log only newly asserted critical faults to prevent kernel log spam
+ * during persistent hardware fault conditions.
+ * dev_*_ratelimited provides an additional safety net against noisy IRQs.
+ */
+ if (!new_faults)
+ return;
+
+ if (new_faults & MC33978_FAULT_OT)
+ dev_crit_ratelimited(priv->dev, "Over-temperature fault detected!\n");
+
+ if (new_faults & MC33978_FAULT_OV)
+ dev_crit_ratelimited(priv->dev, "Over-voltage fault detected!\n");
+
+ if (new_faults & MC33978_FAULT_UV)
+ dev_err_ratelimited(priv->dev, "Under-voltage fault detected!\n");
+}
+
+static irqreturn_t mc33978_hwmon_fault_irq(int irq, void *data)
+{
+ struct mc33978_hwmon_priv *priv = data;
+ u32 faults, new_faults, changed_faults;
+ int ret;
+
+ ret = mc33978_hwmon_read_fault(priv, &faults);
+ if (ret) {
+ dev_err_ratelimited(priv->dev, "Failed to read fault register: %pe\n",
+ ERR_PTR(ret));
+ return IRQ_NONE;
+ }
+
+ changed_faults = faults ^ priv->last_faults;
+ if (!changed_faults)
+ return IRQ_HANDLED;
+
+ new_faults = faults & ~priv->last_faults;
+ if (new_faults)
+ mc33978_hwmon_report_faults(priv, new_faults);
+
+ priv->last_faults = faults;
+
+ if (changed_faults & MC33978_FAULT_UV)
+ hwmon_notify_event(priv->hwmon_dev, hwmon_in,
+ hwmon_in_lcrit_alarm, MC33978_IN_VBATP);
+
+ if (changed_faults & MC33978_FAULT_OV)
+ hwmon_notify_event(priv->hwmon_dev, hwmon_in,
+ hwmon_in_crit_alarm, MC33978_IN_VBATP);
+
+ if (changed_faults & MC33978_FAULT_TEMP_WARN)
+ hwmon_notify_event(priv->hwmon_dev, hwmon_temp,
+ hwmon_temp_max_alarm, 0);
+
+ if (changed_faults & MC33978_FAULT_OT)
+ hwmon_notify_event(priv->hwmon_dev, hwmon_temp,
+ hwmon_temp_crit_alarm, 0);
+
+ /* Push a chip-level alarm on any hardware status change */
+ hwmon_notify_event(priv->hwmon_dev, hwmon_chip,
+ hwmon_chip_alarms, 0);
+
+ return IRQ_HANDLED;
+}
+
+static umode_t mc33978_hwmon_is_visible(const void *data,
+ enum hwmon_sensor_types type,
+ u32 attr, int channel)
+{
+ switch (type) {
+ case hwmon_chip:
+ if (attr == hwmon_chip_alarms)
+ return 0444;
+ break;
+
+ case hwmon_temp:
+ switch (attr) {
+ case hwmon_temp_max:
+ case hwmon_temp_crit:
+ case hwmon_temp_crit_hyst:
+ case hwmon_temp_max_alarm:
+ case hwmon_temp_crit_alarm:
+ return 0444;
+ default:
+ break;
+ }
+ break;
+
+ case hwmon_in:
+ switch (attr) {
+ case hwmon_in_label:
+ case hwmon_in_max:
+ case hwmon_in_min:
+ case hwmon_in_lcrit:
+ return 0444;
+ case hwmon_in_crit:
+ if (channel == MC33978_IN_VBATP)
+ return 0444;
+ break;
+ case hwmon_in_crit_alarm:
+ case hwmon_in_lcrit_alarm:
+ if (channel == MC33978_IN_VBATP)
+ return 0444;
+ break;
+ }
+ break;
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static int mc33978_hwmon_read(struct device *dev,
+ enum hwmon_sensor_types type,
+ u32 attr, int channel, long *val)
+{
+ struct mc33978_hwmon_priv *priv = dev_get_drvdata(dev);
+ u32 faults;
+ int ret;
+
+ switch (type) {
+ case hwmon_in:
+ if (channel == MC33978_IN_VBATP) {
+ switch (attr) {
+ case hwmon_in_crit:
+ *val = MC33978_VBATP_OV_MV;
+ return 0;
+ case hwmon_in_max:
+ *val = MC33978_VBATP_FUNC_MV;
+ return 0;
+ case hwmon_in_min:
+ *val = MC33978_VBATP_DEGRADED_MV;
+ return 0;
+ case hwmon_in_lcrit:
+ *val = MC33978_VBATP_UVLO_MV;
+ return 0;
+ default:
+ break;
+ }
+ } else if (channel == MC33978_IN_VDDQ) {
+ switch (attr) {
+ case hwmon_in_max:
+ *val = MC33978_VDDQ_MAX_MV;
+ return 0;
+ case hwmon_in_min:
+ *val = MC33978_VDDQ_MIN_MV;
+ return 0;
+ case hwmon_in_lcrit:
+ *val = MC33978_VDDQ_UV_MV;
+ return 0;
+ default:
+ break;
+ }
+ }
+ break;
+
+ case hwmon_temp:
+ switch (attr) {
+ case hwmon_temp_max:
+ *val = MC33978_TEMP_WARN_MC;
+ return 0;
+ case hwmon_temp_crit:
+ *val = MC33978_TEMP_CRIT_MC;
+ return 0;
+ case hwmon_temp_crit_hyst:
+ *val = MC33978_TEMP_CRIT_MC - MC33978_TEMP_HYST_MC;
+ return 0;
+ default:
+ break;
+ }
+ break;
+
+ default:
+ break;
+ }
+
+ /* 2. Dynamic alarms (read hardware flags) */
+ ret = mc33978_hwmon_read_fault(priv, &faults);
+ if (ret)
+ return ret;
+
+ switch (type) {
+ case hwmon_chip:
+ if (attr == hwmon_chip_alarms) {
+ *val = faults;
+ return 0;
+ }
+ break;
+
+ case hwmon_in:
+ if (channel == MC33978_IN_VBATP) {
+ switch (attr) {
+ case hwmon_in_crit_alarm:
+ *val = !!(faults & MC33978_FAULT_OV);
+ return 0;
+ case hwmon_in_lcrit_alarm:
+ *val = !!(faults & MC33978_FAULT_UV);
+ return 0;
+ default:
+ *val = 0;
+ return 0;
+ }
+ }
+ /* VDDQ has no dedicated hardware fault flags */
+ *val = 0;
+ return 0;
+
+ case hwmon_temp:
+ switch (attr) {
+ case hwmon_temp_max_alarm:
+ *val = !!(faults & MC33978_FAULT_TEMP_WARN);
+ return 0;
+ case hwmon_temp_crit_alarm:
+ *val = !!(faults & MC33978_FAULT_OT);
+ return 0;
+ default:
+ break;
+ }
+ break;
+
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return -EOPNOTSUPP;
+}
+
+static int mc33978_hwmon_read_string(struct device *dev,
+ enum hwmon_sensor_types type,
+ u32 attr, int channel, const char **str)
+{
+ /* Only in_label is supported for string reads */
+ if (type != hwmon_in || attr != hwmon_in_label)
+ return -EOPNOTSUPP;
+
+ switch (channel) {
+ case MC33978_IN_VBATP:
+ *str = "VBATP";
+ return 0;
+ case MC33978_IN_VDDQ:
+ *str = "VDDQ";
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct hwmon_channel_info * const mc33978_hwmon_info[] = {
+ HWMON_CHANNEL_INFO(chip,
+ HWMON_C_ALARMS),
+ HWMON_CHANNEL_INFO(temp,
+ HWMON_T_MAX | HWMON_T_CRIT | HWMON_T_CRIT_HYST |
+ HWMON_T_MAX_ALARM | HWMON_T_CRIT_ALARM),
+ HWMON_CHANNEL_INFO(in,
+ /* Index 0: MC33978_IN_VBATP */
+ HWMON_I_LABEL | HWMON_I_CRIT | HWMON_I_MAX |
+ HWMON_I_MIN | HWMON_I_LCRIT |
+ HWMON_I_CRIT_ALARM | HWMON_I_LCRIT_ALARM,
+
+ /* Index 1: MC33978_IN_VDDQ */
+ HWMON_I_LABEL | HWMON_I_MAX | HWMON_I_MIN |
+ HWMON_I_LCRIT),
+ NULL
+};
+
+static const struct hwmon_ops mc33978_hwmon_ops = {
+ .is_visible = mc33978_hwmon_is_visible,
+ .read_string = mc33978_hwmon_read_string,
+ .read = mc33978_hwmon_read,
+};
+
+static const struct hwmon_chip_info mc33978_hwmon_chip_info = {
+ .ops = &mc33978_hwmon_ops,
+ .info = mc33978_hwmon_info,
+};
+
+static int mc33978_hwmon_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mc33978_hwmon_priv *priv;
+ struct device *hwmon_dev;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->dev = dev;
+
+ priv->map = dev_get_regmap(dev->parent, NULL);
+ if (!priv->map)
+ return dev_err_probe(dev, -ENODEV, "failed to get regmap\n");
+
+ platform_set_drvdata(pdev, priv);
+
+ priv->fault_irq = platform_get_irq(pdev, 0);
+ if (priv->fault_irq < 0)
+ return priv->fault_irq;
+
+ ret = mc33978_hwmon_read_fault(priv, &priv->last_faults);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to read initial faults\n");
+
+ if (priv->last_faults & MC33978_FAULT_CRITICAL)
+ mc33978_hwmon_report_faults(priv, priv->last_faults);
+
+ hwmon_dev = devm_hwmon_device_register_with_info(dev, "mc33978", priv,
+ &mc33978_hwmon_chip_info,
+ NULL);
+ if (IS_ERR(hwmon_dev))
+ return dev_err_probe(dev, PTR_ERR(hwmon_dev),
+ "failed to register hwmon device\n");
+
+ priv->hwmon_dev = hwmon_dev;
+
+ ret = devm_request_threaded_irq(dev, priv->fault_irq, NULL,
+ mc33978_hwmon_fault_irq, IRQF_ONESHOT,
+ dev_name(dev), priv);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to request fault IRQ\n");
+
+ return 0;
+}
+
+static const struct platform_device_id mc33978_hwmon_id[] = {
+ { "mc33978-hwmon", },
+ { "mc34978-hwmon", },
+ { }
+};
+MODULE_DEVICE_TABLE(platform, mc33978_hwmon_id);
+
+static struct platform_driver mc33978_hwmon_driver = {
+ .driver = {
+ .name = "mc33978-hwmon",
+ },
+ .probe = mc33978_hwmon_probe,
+ .id_table = mc33978_hwmon_id,
+};
+module_platform_driver(mc33978_hwmon_driver);
+
+MODULE_AUTHOR("Oleksij Rempel <kernel@pengutronix.de>");
+MODULE_DESCRIPTION("NXP MC33978/MC34978 Hardware Monitor Driver");
+MODULE_LICENSE("GPL");
--
2.47.3
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 6/6] mux: add NXP MC33978/MC34978 AMUX driver
2026-03-03 13:39 [PATCH v2 0/8] mfd: Add support for NXP MC33978/MC34978 MSDI Oleksij Rempel
` (4 preceding siblings ...)
2026-03-03 13:39 ` [PATCH v2 5/6] hwmon: add NXP MC33978/MC34978 driver Oleksij Rempel
@ 2026-03-03 13:39 ` Oleksij Rempel
5 siblings, 0 replies; 25+ messages in thread
From: Oleksij Rempel @ 2026-03-03 13:39 UTC (permalink / raw)
To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lee Jones, Peter Rosin, Linus Walleij
Cc: Oleksij Rempel, kernel, linux-kernel, devicetree, linux-hwmon,
linux-gpio, David Jander
Add a mux-control driver for the 24-to-1 analog multiplexer (AMUX)
embedded in the NXP MC33978/MC34978 Multiple Switch Detection
Interface (MSDI) devices.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v2:
- Add missing <linux/err.h> include.
- Add platform_device_id table
---
drivers/mux/Kconfig | 14 +++++
drivers/mux/Makefile | 2 +
drivers/mux/mc33978-mux.c | 120 ++++++++++++++++++++++++++++++++++++++
3 files changed, 136 insertions(+)
create mode 100644 drivers/mux/mc33978-mux.c
diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
index c68132e38138..7532da7e087e 100644
--- a/drivers/mux/Kconfig
+++ b/drivers/mux/Kconfig
@@ -45,6 +45,20 @@ config MUX_GPIO
To compile the driver as a module, choose M here: the module will
be called mux-gpio.
+config MUX_MC33978
+ tristate "NXP MC33978/MC34978 Analog Multiplexer"
+ depends on MFD_MC33978
+ help
+ MC33978/MC34978 24-to-1 analog multiplexer (AMUX) driver.
+
+ This driver provides mux-control for the analog multiplexer,
+ which can route switch voltages, temperature, and battery voltage
+ to an external ADC. Typically used with IIO ADC drivers to measure
+ analog values from the 22 switch inputs plus temperature and VBATP.
+
+ To compile the driver as a module, choose M here: the module will
+ be called mc33978-mux.
+
config MUX_MMIO
tristate "MMIO/Regmap register bitfield-controlled Multiplexer"
depends on OF
diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
index 6e9fa47daf56..339c44b4d4f4 100644
--- a/drivers/mux/Makefile
+++ b/drivers/mux/Makefile
@@ -7,10 +7,12 @@ mux-core-objs := core.o
mux-adg792a-objs := adg792a.o
mux-adgs1408-objs := adgs1408.o
mux-gpio-objs := gpio.o
+mux-mc33978-objs := mc33978-mux.o
mux-mmio-objs := mmio.o
obj-$(CONFIG_MULTIPLEXER) += mux-core.o
obj-$(CONFIG_MUX_ADG792A) += mux-adg792a.o
obj-$(CONFIG_MUX_ADGS1408) += mux-adgs1408.o
obj-$(CONFIG_MUX_GPIO) += mux-gpio.o
+obj-$(CONFIG_MUX_MC33978) += mux-mc33978.o
obj-$(CONFIG_MUX_MMIO) += mux-mmio.o
diff --git a/drivers/mux/mc33978-mux.c b/drivers/mux/mc33978-mux.c
new file mode 100644
index 000000000000..47bf6e146287
--- /dev/null
+++ b/drivers/mux/mc33978-mux.c
@@ -0,0 +1,120 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Copyright (c) 2026 Pengutronix, Oleksij Rempel <kernel@pengutronix.de>
+/*
+ * MC33978/MC34978 Analog Multiplexer (AMUX) Driver
+ *
+ * This driver provides mux-control for the 24-to-1 analog multiplexer.
+ * The AMUX routes one of the following signals to the external AMUX pin:
+ * - Channels 0-13: SG0-SG13 switch voltages
+ * - Channels 14-21: SP0-SP7 switch voltages
+ * - Channel 22: Internal temperature diode
+ * - Channel 23: Battery voltage (VBATP)
+ *
+ * Consumer drivers (typically IIO ADC drivers) use the mux-control
+ * subsystem to select which signal to measure.
+ *
+ * Architecture:
+ * ============
+ * The MC33978 does not have an internal ADC. Instead, it routes analog
+ * signals to an external AMUX pin that must be connected to an external
+ * ADC (such as the SoC's internal ADC). The IIO subsystem is responsible
+ * for coordinating the mux selection and ADC sampling.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mux/driver.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+#include <linux/mfd/mc33978.h>
+
+/* AMUX_CTRL register field definitions */
+#define MC33978_AMUX_CTRL_MASK GENMASK(5, 0) /* 6-bit channel select */
+
+struct mc33978_mux_priv {
+ struct device *dev;
+ struct regmap *map;
+};
+
+static int mc33978_mux_set(struct mux_control *mux, int state)
+{
+ struct mc33978_mux_priv *priv = mux_chip_priv(mux->chip);
+ int ret;
+
+ if (state < 0 || state >= MC33978_NUM_AMUX_CH)
+ return -EINVAL;
+
+ ret = regmap_update_bits(priv->map, MC33978_REG_AMUX_CTRL,
+ MC33978_AMUX_CTRL_MASK, state);
+ if (ret) {
+ dev_err(priv->dev, "Failed to set AMUX channel %d: %d\n",
+ state, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct mux_control_ops mc33978_mux_ops = {
+ .set = mc33978_mux_set,
+};
+
+static int mc33978_mux_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mc33978_mux_priv *priv;
+ struct mux_chip *mux_chip;
+ struct mux_control *mux;
+ int ret;
+
+ mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*priv));
+ if (IS_ERR(mux_chip))
+ return dev_err_probe(dev, PTR_ERR(mux_chip), "Failed to allocate mux chip\n");
+
+ /* Borrow the parent's DT node so consumers can find this mux chip */
+ device_set_node(&mux_chip->dev, dev_fwnode(dev->parent));
+
+ priv = mux_chip_priv(mux_chip);
+ priv->dev = dev;
+
+ priv->map = dev_get_regmap(dev->parent, NULL);
+ if (!priv->map)
+ return dev_err_probe(dev, -ENODEV, "Failed to get parent regmap\n");
+
+ mux_chip->ops = &mc33978_mux_ops;
+
+ mux = &mux_chip->mux[0];
+ mux->states = MC33978_NUM_AMUX_CH;
+
+ ret = devm_mux_chip_register(dev, mux_chip);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to register mux chip\n");
+
+ platform_set_drvdata(pdev, mux_chip);
+
+ return 0;
+}
+
+static const struct platform_device_id mc33978_mux_id[] = {
+ { "mc33978-mux", },
+ { "mc34978-mux", },
+ { }
+};
+MODULE_DEVICE_TABLE(platform, mc33978_mux_id);
+
+static struct platform_driver mc33978_mux_driver = {
+ .driver = {
+ .name = "mc33978-mux",
+ },
+ .probe = mc33978_mux_probe,
+ .id_table = mc33978_mux_id,
+};
+module_platform_driver(mc33978_mux_driver);
+
+MODULE_AUTHOR("Oleksij Rempel <kernel@pengutronix.de>");
+MODULE_DESCRIPTION("NXP MC33978/MC34978 Analog Multiplexer Driver");
+MODULE_LICENSE("GPL");
--
2.47.3
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: mfd: add NXP MC33978/MC34978 MSDI
2026-03-03 13:39 ` [PATCH v2 1/6] dt-bindings: mfd: add " Oleksij Rempel
@ 2026-03-03 14:40 ` Rob Herring (Arm)
2026-03-03 16:10 ` Oleksij Rempel
2026-03-04 7:59 ` Krzysztof Kozlowski
1 sibling, 1 reply; 25+ messages in thread
From: Rob Herring (Arm) @ 2026-03-03 14:40 UTC (permalink / raw)
To: Oleksij Rempel
Cc: devicetree, Krzysztof Kozlowski, Conor Dooley, Peter Rosin,
kernel, linux-kernel, linux-gpio, David Jander, Lee Jones,
Guenter Roeck, Linus Walleij, linux-hwmon
On Tue, 03 Mar 2026 14:39:41 +0100, Oleksij Rempel wrote:
> Add device tree binding documentation for the NXP MC33978 and MC34978
> Multiple Switch Detection Interface (MSDI) devices.
>
> These ICs monitor up to 22 mechanical switch contacts in automotive and
> industrial environments. They provide configurable wetting currents to
> break through contact oxidation and feature extensive hardware
> protection against thermal overload and voltage transients (load
> dumps/brown-outs).
>
> The device interfaces via SPI and provides multiple functions. To
> accurately represent the hardware without unnecessary DT overhead, the
> binding is structured as follows:
> - pinctrl: A dedicated child node managing the 22 switch inputs (SG/SP
> pins) and their GPIO configurations.
> - hwmon: Integrated into the parent node, exposing critical hardware
> faults (OT, OV, UV) and static voltage/temperature thresholds.
> - mux: Integrated into the parent node, controlling the 24-to-1 analog
> multiplexer to route pin voltages, internal temperature, or battery
> voltage to an external SoC ADC.
>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
> changes v2:
> - Squashed MFD, pinctrl, hwmon, and mux bindings into a single patch
> - Removed the empty hwmon child node
> - Folded the mux-controller node into the parent MFD node
> - Added vbatp-supply and vddq-supply to the required properties block
> - Changed the example node name from mc33978@0 to gpio@0
> - Removed unnecessary literal block scalars (|) from descriptions
> - Documented SG, SP, and SB pin acronyms in the pinctrl description
> - Added consumer polarity guidance (GPIO_ACTIVE_LOW/HIGH) for SG/SB
> inputs, with a note on output circuit dependency
> - Updated commit message
> ---
> .../devicetree/bindings/mfd/nxp,mc33978.yaml | 114 ++++++++++++++++++
> .../bindings/pinctrl/nxp,mc33978-pinctrl.yaml | 82 +++++++++++++
> 2 files changed, 196 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/nxp,mc33978.yaml
> create mode 100644 Documentation/devicetree/bindings/pinctrl/nxp,mc33978-pinctrl.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/nxp,mc33978.example.dtb: gpio@0 (nxp,mc33978): $nodename:0: 'gpio@0' does not match '^mux-controller(@.*|-([0-9]|[1-9][0-9]+))?$'
from schema $id: http://devicetree.org/schemas/mux/mux-controller.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.kernel.org/project/devicetree/patch/20260303133947.1123575-2-o.rempel@pengutronix.de
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: mfd: add NXP MC33978/MC34978 MSDI
2026-03-03 14:40 ` Rob Herring (Arm)
@ 2026-03-03 16:10 ` Oleksij Rempel
2026-03-04 8:05 ` Krzysztof Kozlowski
2026-03-04 18:26 ` Rob Herring
0 siblings, 2 replies; 25+ messages in thread
From: Oleksij Rempel @ 2026-03-03 16:10 UTC (permalink / raw)
To: Rob Herring (Arm)
Cc: devicetree, Krzysztof Kozlowski, Conor Dooley, Peter Rosin,
kernel, linux-kernel, linux-gpio, David Jander, Lee Jones,
Guenter Roeck, Linus Walleij, linux-hwmon
Hi Krzysztof and Rob,
On Tue, Mar 03, 2026 at 08:40:55AM -0600, Rob Herring (Arm) wrote:
> > .../devicetree/bindings/mfd/nxp,mc33978.yaml | 114 ++++++++++++++++++
> > .../bindings/pinctrl/nxp,mc33978-pinctrl.yaml | 82 +++++++++++++
> > 2 files changed, 196 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/mfd/nxp,mc33978.yaml
> > create mode 100644 Documentation/devicetree/bindings/pinctrl/nxp,mc33978-pinctrl.yaml
> >
>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/nxp,mc33978.example.dtb: gpio@0 (nxp,mc33978): $nodename:0: 'gpio@0' does not match '^mux-controller(@.*|-([0-9]|[1-9][0-9]+))?$'
> from schema $id: http://devicetree.org/schemas/mux/mux-controller.yaml
>
Folding the mux node into the parent as suggested [1] causes this error.
Because the parent now has #mux-control-cells, the generic
mux-controller.yaml forces the node name to be mux-controller. Since
this chip is primarily a switch/GPIO controller, naming the parent SPI
node mux-controller@0 is misleading.
What is the preferred way to go here?
[1] https://lore.kernel.org/all/20260226-clever-rustling-dolphin-871aff@quoll/
Best Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: mfd: add NXP MC33978/MC34978 MSDI
2026-03-03 13:39 ` [PATCH v2 1/6] dt-bindings: mfd: add " Oleksij Rempel
2026-03-03 14:40 ` Rob Herring (Arm)
@ 2026-03-04 7:59 ` Krzysztof Kozlowski
1 sibling, 0 replies; 25+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-04 7:59 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lee Jones, Peter Rosin, Linus Walleij, kernel, linux-kernel,
devicetree, linux-hwmon, linux-gpio, David Jander
On Tue, Mar 03, 2026 at 02:39:41PM +0100, Oleksij Rempel wrote:
> + '#mux-control-cells':
> + const: 0
> + description:
> + Present if the device AMUX selector is used as a mux provider.
> + Consumers (e.g. io-channel-mux) must provide settle-time-us for the
> + external ADC sampling path.
> +
> + vddq-supply:
> + description: Digital supply voltage
> +
> + vbatp-supply:
> + description: Battery/power supply
> +
> +patternProperties:
> + "^pinctrl(@.*)?":
Drop @ part. Your binding does not allow addressing anyway.
> + type: object
> + $ref: /schemas/pinctrl/nxp,mc33978-pinctrl.yaml#
> + description:
> + Pinctrl and GPIO controller child node for the 22 switch inputs.
> +
> +required:
> + - compatible
> + - interrupt-controller
> + - '#interrupt-cells'
> + - interrupts
> + - reg
Odd order. Keep the same as in the list of properties.
> + - vbatp-supply
> + - vddq-supply
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + #include <dt-bindings/gpio/gpio.h>
> +
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + msdi: gpio@0 {
> + compatible = "nxp,mc33978";
> + reg = <0>;
> + spi-max-frequency = <4000000>;
> +
> + interrupt-parent = <&gpiog>;
> + interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> +
> + vddq-supply = <®_3v3>;
> + vbatp-supply = <®_12v>;
> +
> + #mux-control-cells = <0>;
> +
> + pinctrl {
> + compatible = "nxp,mc33978-pinctrl";
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> +
Stray blank line
> + };
> + };
> diff --git a/Documentation/devicetree/bindings/pinctrl/nxp,mc33978-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nxp,mc33978-pinctrl.yaml
> new file mode 100644
> index 000000000000..f8257d55d466
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/nxp,mc33978-pinctrl.yaml
> @@ -0,0 +1,82 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/nxp,mc33978-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP MC33978/MC34978 Pinctrl/GPIO Driver
> +
> +maintainers:
> + - David Jander <david@protonic.nl>
> + - Oleksij Rempel <o.rempel@pengutronix.de>
> +
> +description: |
> + Pin control and GPIO driver for the MC33978/MC34978 MSDI device.
> +
> + Pin numbering:
> + - Pins 0-13: SG0-SG13 (Switch-to-Ground inputs). These pins monitor
> + contacts closed to ground and typically require GPIO_ACTIVE_LOW
> + flags when used as digital inputs.
> + - Pins 14-21: SP0-SP7 (Programmable inputs). These can be configured
> + as SG (Switch-to-Ground) or SB (Switch-to-Battery) inputs. SB
> + inputs monitor contacts closed to the battery voltage and typically
> + require GPIO_ACTIVE_HIGH flags when used as digital inputs.
> +
> + Output Emulation:
> + The hardware lacks standard push-pull output drivers. Outputs are emulated
> + by toggling the programmable wetting current sources (acting as pull-ups or
> + pull-downs) and the hardware tri-state registers. Because of this physical
> + constraint:
> + - Consumers using pins as outputs MUST flag them with GPIO_OPEN_DRAIN or
> + GPIO_OPEN_SOURCE in the device tree.
> + - Push-pull configurations are physically unsupported.
> + - The active polarity depends entirely on the external circuit (e.g., how an
> + LED is wired) and must be flagged accordingly by the consumer.
> +
> +properties:
> + compatible:
> + enum:
> + - nxp,mc33978-pinctrl
> + - nxp,mc34978-pinctrl
> +
> + gpio-controller: true
> +
> + '#gpio-cells':
> + const: 2
> +
> + ngpios:
> + const: 22
> +
> +patternProperties:
> + "^.*-grp$":
> + type: object
> + $ref: pincfg-node.yaml#
> + additionalProperties: false
> + description:
> + Pin configuration subnodes.
> + properties:
> + pins: true
> + bias-pull-up: true
> + bias-pull-down: true
> + bias-high-impedance: true
> +
> +required:
> + - compatible
> + - gpio-controller
> + - '#gpio-cells'
> +
> +unevaluatedProperties: false
additionalProperties instead
> +
> +examples:
> + - |
> + pinctrl {
> + compatible = "nxp,mc33978-pinctrl";
> + gpio-controller;
> + #gpio-cells = <2>;
> + ngpios = <22>;
> +
> + door-grp {
> + pins = "sg0";
> + bias-high-impedance;
> + };
> + };
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: mfd: add NXP MC33978/MC34978 MSDI
2026-03-03 16:10 ` Oleksij Rempel
@ 2026-03-04 8:05 ` Krzysztof Kozlowski
2026-03-04 9:06 ` David Jander
2026-03-04 18:26 ` Rob Herring
1 sibling, 1 reply; 25+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-04 8:05 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Rob Herring (Arm), devicetree, Krzysztof Kozlowski, Conor Dooley,
Peter Rosin, kernel, linux-kernel, linux-gpio, David Jander,
Lee Jones, Guenter Roeck, Linus Walleij, linux-hwmon
On Tue, Mar 03, 2026 at 05:10:20PM +0100, Oleksij Rempel wrote:
> Hi Krzysztof and Rob,
>
> On Tue, Mar 03, 2026 at 08:40:55AM -0600, Rob Herring (Arm) wrote:
> > > .../devicetree/bindings/mfd/nxp,mc33978.yaml | 114 ++++++++++++++++++
> > > .../bindings/pinctrl/nxp,mc33978-pinctrl.yaml | 82 +++++++++++++
> > > 2 files changed, 196 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/mfd/nxp,mc33978.yaml
> > > create mode 100644 Documentation/devicetree/bindings/pinctrl/nxp,mc33978-pinctrl.yaml
> > >
> >
> > My bot found errors running 'make dt_binding_check' on your patch:
> >
> > yamllint warnings/errors:
> >
> > dtschema/dtc warnings/errors:
> > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/nxp,mc33978.example.dtb: gpio@0 (nxp,mc33978): $nodename:0: 'gpio@0' does not match '^mux-controller(@.*|-([0-9]|[1-9][0-9]+))?$'
> > from schema $id: http://devicetree.org/schemas/mux/mux-controller.yaml
> >
>
> Folding the mux node into the parent as suggested [1] causes this error.
> Because the parent now has #mux-control-cells, the generic
> mux-controller.yaml forces the node name to be mux-controller. Since
> this chip is primarily a switch/GPIO controller, naming the parent SPI
> node mux-controller@0 is misleading.
>
> What is the preferred way to go here?
https://www.nxp.com/products/interfaces/multi-switch-detection-interface/22-i-o-msdi-programmable-current-analog-mux:MC33978
Name of the mc33978 device is "programmable analog mux" and further
description says "analog multiplexer for reading analog inputs ", so I
don't find "mux-controller" a confusing name. It is EXACTLY a
mux, so mux-controller.
Anyway if you want gpio, then please add a patch extending the pattern
in mux-controller.yaml to allow "gpio".
Alternative, because it is rather a mux than a controller of a mux,
would be to call it just "mux" or "io-mux" (maybe the latter, since we
have "i2c-mux" in the spec) and allow that pattern to be in
mux-controller.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: mfd: add NXP MC33978/MC34978 MSDI
2026-03-04 8:05 ` Krzysztof Kozlowski
@ 2026-03-04 9:06 ` David Jander
2026-03-04 9:49 ` Krzysztof Kozlowski
0 siblings, 1 reply; 25+ messages in thread
From: David Jander @ 2026-03-04 9:06 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Oleksij Rempel, Rob Herring (Arm), devicetree,
Krzysztof Kozlowski, Conor Dooley, Peter Rosin, kernel,
linux-kernel, linux-gpio, Lee Jones, Guenter Roeck, Linus Walleij,
linux-hwmon
Hi Krzysztof,
On Wed, 4 Mar 2026 09:05:11 +0100
Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Tue, Mar 03, 2026 at 05:10:20PM +0100, Oleksij Rempel wrote:
> > Hi Krzysztof and Rob,
> >
> > On Tue, Mar 03, 2026 at 08:40:55AM -0600, Rob Herring (Arm) wrote:
> > > > .../devicetree/bindings/mfd/nxp,mc33978.yaml | 114 ++++++++++++++++++
> > > > .../bindings/pinctrl/nxp,mc33978-pinctrl.yaml | 82 +++++++++++++
> > > > 2 files changed, 196 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/mfd/nxp,mc33978.yaml
> > > > create mode 100644 Documentation/devicetree/bindings/pinctrl/nxp,mc33978-pinctrl.yaml
> > > >
> > >
> > > My bot found errors running 'make dt_binding_check' on your patch:
> > >
> > > yamllint warnings/errors:
> > >
> > > dtschema/dtc warnings/errors:
> > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/nxp,mc33978.example.dtb: gpio@0 (nxp,mc33978): $nodename:0: 'gpio@0' does not match '^mux-controller(@.*|-([0-9]|[1-9][0-9]+))?$'
> > > from schema $id: http://devicetree.org/schemas/mux/mux-controller.yaml
> > >
> >
> > Folding the mux node into the parent as suggested [1] causes this error.
> > Because the parent now has #mux-control-cells, the generic
> > mux-controller.yaml forces the node name to be mux-controller. Since
> > this chip is primarily a switch/GPIO controller, naming the parent SPI
> > node mux-controller@0 is misleading.
> >
> > What is the preferred way to go here?
>
> https://www.nxp.com/products/interfaces/multi-switch-detection-interface/22-i-o-msdi-programmable-current-analog-mux:MC33978
>
> Name of the mc33978 device is "programmable analog mux" and further
> description says "analog multiplexer for reading analog inputs ", so I
> don't find "mux-controller" a confusing name. It is EXACTLY a
> mux, so mux-controller.
Sorry to chime in here. I'm afraid the NXP description on that link you posted
is a typo. It is not correct. This chip is primarily a "Switch Detection
Interface", or in other wordt a switch input interface. Wee here for the same
page for the MC34978, which is the exact same chip:
https://www.nxp.com/products/interfaces/multi-switch-detection-interface/switch-detection-interface-22-i-os-programmable-wetting-current-temp-sensor-3-3-v-5-0-v-spi:MC34978
It has an additional function that can be used as an analog MUX, but it is an
extra feature and definitely NOT its primary function.
Not sure if this is relevant, but I fear there might be some confusion.
Best regards,
> Anyway if you want gpio, then please add a patch extending the pattern
> in mux-controller.yaml to allow "gpio".
>
> Alternative, because it is rather a mux than a controller of a mux,
> would be to call it just "mux" or "io-mux" (maybe the latter, since we
> have "i2c-mux" in the spec) and allow that pattern to be in
> mux-controller.
>
>
> Best regards,
> Krzysztof
--
David Jander
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: mfd: add NXP MC33978/MC34978 MSDI
2026-03-04 9:06 ` David Jander
@ 2026-03-04 9:49 ` Krzysztof Kozlowski
2026-03-04 10:25 ` David Jander
0 siblings, 1 reply; 25+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-04 9:49 UTC (permalink / raw)
To: David Jander
Cc: Oleksij Rempel, Rob Herring (Arm), devicetree,
Krzysztof Kozlowski, Conor Dooley, Peter Rosin, kernel,
linux-kernel, linux-gpio, Lee Jones, Guenter Roeck, Linus Walleij,
linux-hwmon
On 04/03/2026 10:06, David Jander wrote:
>
> Hi Krzysztof,
>
> On Wed, 4 Mar 2026 09:05:11 +0100
> Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
>> On Tue, Mar 03, 2026 at 05:10:20PM +0100, Oleksij Rempel wrote:
>>> Hi Krzysztof and Rob,
>>>
>>> On Tue, Mar 03, 2026 at 08:40:55AM -0600, Rob Herring (Arm) wrote:
>>>>> .../devicetree/bindings/mfd/nxp,mc33978.yaml | 114 ++++++++++++++++++
>>>>> .../bindings/pinctrl/nxp,mc33978-pinctrl.yaml | 82 +++++++++++++
>>>>> 2 files changed, 196 insertions(+)
>>>>> create mode 100644 Documentation/devicetree/bindings/mfd/nxp,mc33978.yaml
>>>>> create mode 100644 Documentation/devicetree/bindings/pinctrl/nxp,mc33978-pinctrl.yaml
>>>>>
>>>>
>>>> My bot found errors running 'make dt_binding_check' on your patch:
>>>>
>>>> yamllint warnings/errors:
>>>>
>>>> dtschema/dtc warnings/errors:
>>>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/nxp,mc33978.example.dtb: gpio@0 (nxp,mc33978): $nodename:0: 'gpio@0' does not match '^mux-controller(@.*|-([0-9]|[1-9][0-9]+))?$'
>>>> from schema $id: http://devicetree.org/schemas/mux/mux-controller.yaml
>>>>
>>>
>>> Folding the mux node into the parent as suggested [1] causes this error.
>>> Because the parent now has #mux-control-cells, the generic
>>> mux-controller.yaml forces the node name to be mux-controller. Since
>>> this chip is primarily a switch/GPIO controller, naming the parent SPI
>>> node mux-controller@0 is misleading.
>>>
>>> What is the preferred way to go here?
>>
>> https://www.nxp.com/products/interfaces/multi-switch-detection-interface/22-i-o-msdi-programmable-current-analog-mux:MC33978
>>
>> Name of the mc33978 device is "programmable analog mux" and further
>> description says "analog multiplexer for reading analog inputs ", so I
>> don't find "mux-controller" a confusing name. It is EXACTLY a
>> mux, so mux-controller.
>
> Sorry to chime in here. I'm afraid the NXP description on that link you posted
> is a typo. It is not correct. This chip is primarily a "Switch Detection
> Interface", or in other wordt a switch input interface. Wee here for the same
> page for the MC34978, which is the exact same chip:
>
> https://www.nxp.com/products/interfaces/multi-switch-detection-interface/switch-detection-interface-22-i-os-programmable-wetting-current-temp-sensor-3-3-v-5-0-v-spi:MC34978
That's MC34978 and I commented on MC33978.
What is the primary function of MC33978 being described here as the base?
>
> It has an additional function that can be used as an analog MUX, but it is an
> extra feature and definitely NOT its primary function.
>
> Not sure if this is relevant, but I fear there might be some confusion.
>
> Best regards,
>
>> Anyway if you want gpio, then please add a patch extending the pattern
>> in mux-controller.yaml to allow "gpio".
>>
>> Alternative, because it is rather a mux than a controller of a mux,
>> would be to call it just "mux" or "io-mux" (maybe the latter, since we
>> have "i2c-mux" in the spec) and allow that pattern to be in
>> mux-controller.
>>
>>
>> Best regards,
>> Krzysztof
>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: mfd: add NXP MC33978/MC34978 MSDI
2026-03-04 9:49 ` Krzysztof Kozlowski
@ 2026-03-04 10:25 ` David Jander
2026-03-04 11:41 ` Krzysztof Kozlowski
0 siblings, 1 reply; 25+ messages in thread
From: David Jander @ 2026-03-04 10:25 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Oleksij Rempel, Rob Herring (Arm), devicetree,
Krzysztof Kozlowski, Conor Dooley, Peter Rosin, kernel,
linux-kernel, linux-gpio, Lee Jones, Guenter Roeck, Linus Walleij,
linux-hwmon
On Wed, 4 Mar 2026 10:49:06 +0100
Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On 04/03/2026 10:06, David Jander wrote:
> >
> > Hi Krzysztof,
> >
> > On Wed, 4 Mar 2026 09:05:11 +0100
> > Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> >> On Tue, Mar 03, 2026 at 05:10:20PM +0100, Oleksij Rempel wrote:
> >>> Hi Krzysztof and Rob,
> >>>
> >>> On Tue, Mar 03, 2026 at 08:40:55AM -0600, Rob Herring (Arm) wrote:
> >>>>> .../devicetree/bindings/mfd/nxp,mc33978.yaml | 114 ++++++++++++++++++
> >>>>> .../bindings/pinctrl/nxp,mc33978-pinctrl.yaml | 82 +++++++++++++
> >>>>> 2 files changed, 196 insertions(+)
> >>>>> create mode 100644 Documentation/devicetree/bindings/mfd/nxp,mc33978.yaml
> >>>>> create mode 100644 Documentation/devicetree/bindings/pinctrl/nxp,mc33978-pinctrl.yaml
> >>>>>
> >>>>
> >>>> My bot found errors running 'make dt_binding_check' on your patch:
> >>>>
> >>>> yamllint warnings/errors:
> >>>>
> >>>> dtschema/dtc warnings/errors:
> >>>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/nxp,mc33978.example.dtb: gpio@0 (nxp,mc33978): $nodename:0: 'gpio@0' does not match '^mux-controller(@.*|-([0-9]|[1-9][0-9]+))?$'
> >>>> from schema $id: http://devicetree.org/schemas/mux/mux-controller.yaml
> >>>>
> >>>
> >>> Folding the mux node into the parent as suggested [1] causes this error.
> >>> Because the parent now has #mux-control-cells, the generic
> >>> mux-controller.yaml forces the node name to be mux-controller. Since
> >>> this chip is primarily a switch/GPIO controller, naming the parent SPI
> >>> node mux-controller@0 is misleading.
> >>>
> >>> What is the preferred way to go here?
> >>
> >> https://www.nxp.com/products/interfaces/multi-switch-detection-interface/22-i-o-msdi-programmable-current-analog-mux:MC33978
> >>
> >> Name of the mc33978 device is "programmable analog mux" and further
> >> description says "analog multiplexer for reading analog inputs ", so I
> >> don't find "mux-controller" a confusing name. It is EXACTLY a
> >> mux, so mux-controller.
> >
> > Sorry to chime in here. I'm afraid the NXP description on that link you posted
> > is a typo. It is not correct. This chip is primarily a "Switch Detection
> > Interface", or in other wordt a switch input interface. Wee here for the same
> > page for the MC34978, which is the exact same chip:
> >
> > https://www.nxp.com/products/interfaces/multi-switch-detection-interface/switch-detection-interface-22-i-os-programmable-wetting-current-temp-sensor-3-3-v-5-0-v-spi:MC34978
>
> That's MC34978 and I commented on MC33978.
>
> What is the primary function of MC33978 being described here as the base?
The MC34978 and MC33978 are the exact same part (except for the temperature
range). The fact that NXP has two different web-pages with two different
descriptions of it certainly doesn't help, but you can also check the
datasheet[1] description: "MC33978: 22-channel multiple switch detection
interface with programmable wetting current"
Further down in the description it says: "It also features a 24-to-1 analog
multiplexer for reading inputs as analog."
IMHO this makes it clear that this is NOT primarily a MUX.
Actually, I doubt many users of this chip will use the analog MUX function at
all since it has quite a few limitations that make it not very practical to
use.
The most fitting Linux framework for this chip's primary funtcion IMHO is
pinctrl/gpio, but there are some caveats unfortunately.
[1] https://www.nxp.com/docs/en/data-sheet/MC33978.pdf
Best regards,
> > It has an additional function that can be used as an analog MUX, but it is an
> > extra feature and definitely NOT its primary function.
> >
> > Not sure if this is relevant, but I fear there might be some confusion.
> >
> > Best regards,
> >
> >> Anyway if you want gpio, then please add a patch extending the pattern
> >> in mux-controller.yaml to allow "gpio".
> >>
> >> Alternative, because it is rather a mux than a controller of a mux,
> >> would be to call it just "mux" or "io-mux" (maybe the latter, since we
> >> have "i2c-mux" in the spec) and allow that pattern to be in
> >> mux-controller.
--
David Jander
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: mfd: add NXP MC33978/MC34978 MSDI
2026-03-04 10:25 ` David Jander
@ 2026-03-04 11:41 ` Krzysztof Kozlowski
2026-03-04 12:17 ` David Jander
0 siblings, 1 reply; 25+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-04 11:41 UTC (permalink / raw)
To: David Jander
Cc: Oleksij Rempel, Rob Herring (Arm), devicetree,
Krzysztof Kozlowski, Conor Dooley, Peter Rosin, kernel,
linux-kernel, linux-gpio, Lee Jones, Guenter Roeck, Linus Walleij,
linux-hwmon
On 04/03/2026 11:25, David Jander wrote:
> On Wed, 4 Mar 2026 10:49:06 +0100
> Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
>> On 04/03/2026 10:06, David Jander wrote:
>>>
>>> Hi Krzysztof,
>>>
>>> On Wed, 4 Mar 2026 09:05:11 +0100
>>> Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>
>>>> On Tue, Mar 03, 2026 at 05:10:20PM +0100, Oleksij Rempel wrote:
>>>>> Hi Krzysztof and Rob,
>>>>>
>>>>> On Tue, Mar 03, 2026 at 08:40:55AM -0600, Rob Herring (Arm) wrote:
>>>>>>> .../devicetree/bindings/mfd/nxp,mc33978.yaml | 114 ++++++++++++++++++
>>>>>>> .../bindings/pinctrl/nxp,mc33978-pinctrl.yaml | 82 +++++++++++++
>>>>>>> 2 files changed, 196 insertions(+)
>>>>>>> create mode 100644 Documentation/devicetree/bindings/mfd/nxp,mc33978.yaml
>>>>>>> create mode 100644 Documentation/devicetree/bindings/pinctrl/nxp,mc33978-pinctrl.yaml
>>>>>>>
>>>>>>
>>>>>> My bot found errors running 'make dt_binding_check' on your patch:
>>>>>>
>>>>>> yamllint warnings/errors:
>>>>>>
>>>>>> dtschema/dtc warnings/errors:
>>>>>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/nxp,mc33978.example.dtb: gpio@0 (nxp,mc33978): $nodename:0: 'gpio@0' does not match '^mux-controller(@.*|-([0-9]|[1-9][0-9]+))?$'
>>>>>> from schema $id: http://devicetree.org/schemas/mux/mux-controller.yaml
>>>>>>
>>>>>
>>>>> Folding the mux node into the parent as suggested [1] causes this error.
>>>>> Because the parent now has #mux-control-cells, the generic
>>>>> mux-controller.yaml forces the node name to be mux-controller. Since
>>>>> this chip is primarily a switch/GPIO controller, naming the parent SPI
>>>>> node mux-controller@0 is misleading.
>>>>>
>>>>> What is the preferred way to go here?
>>>>
>>>> https://www.nxp.com/products/interfaces/multi-switch-detection-interface/22-i-o-msdi-programmable-current-analog-mux:MC33978
>>>>
>>>> Name of the mc33978 device is "programmable analog mux" and further
>>>> description says "analog multiplexer for reading analog inputs ", so I
>>>> don't find "mux-controller" a confusing name. It is EXACTLY a
>>>> mux, so mux-controller.
>>>
>>> Sorry to chime in here. I'm afraid the NXP description on that link you posted
>>> is a typo. It is not correct. This chip is primarily a "Switch Detection
>>> Interface", or in other wordt a switch input interface. Wee here for the same
>>> page for the MC34978, which is the exact same chip:
>>>
>>> https://www.nxp.com/products/interfaces/multi-switch-detection-interface/switch-detection-interface-22-i-os-programmable-wetting-current-temp-sensor-3-3-v-5-0-v-spi:MC34978
>>
>> That's MC34978 and I commented on MC33978.
>>
>> What is the primary function of MC33978 being described here as the base?
>
> The MC34978 and MC33978 are the exact same part (except for the temperature
> range). The fact that NXP has two different web-pages with two different
> descriptions of it certainly doesn't help, but you can also check the
> datasheet[1] description: "MC33978: 22-channel multiple switch detection
> interface with programmable wetting current"
>
> Further down in the description it says: "It also features a 24-to-1 analog
> multiplexer for reading inputs as analog."
> IMHO this makes it clear that this is NOT primarily a MUX.
>
> Actually, I doubt many users of this chip will use the analog MUX function at
> all since it has quite a few limitations that make it not very practical to
> use.
>
> The most fitting Linux framework for this chip's primary funtcion IMHO is
> pinctrl/gpio, but there are some caveats unfortunately.
>
> [1] https://www.nxp.com/docs/en/data-sheet/MC33978.pdf
OK, thanks for the explanation, but then primary function is not GPIO
either, because nothing on linked page says it is a generic purpose IO.
It says it is switch detection. Maybe better generic name is then
"pinctrl", thus also "pinctrl" child should be folded into the parent...
but switch detection is also not a pinctrl. :/
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: mfd: add NXP MC33978/MC34978 MSDI
2026-03-04 11:41 ` Krzysztof Kozlowski
@ 2026-03-04 12:17 ` David Jander
2026-03-05 12:54 ` Linus Walleij
0 siblings, 1 reply; 25+ messages in thread
From: David Jander @ 2026-03-04 12:17 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Oleksij Rempel, Rob Herring (Arm), devicetree,
Krzysztof Kozlowski, Conor Dooley, Peter Rosin, kernel,
linux-kernel, linux-gpio, Lee Jones, Guenter Roeck, Linus Walleij,
linux-hwmon
On Wed, 4 Mar 2026 12:41:37 +0100
Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On 04/03/2026 11:25, David Jander wrote:
> > On Wed, 4 Mar 2026 10:49:06 +0100
> > Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> >> On 04/03/2026 10:06, David Jander wrote:
> >>>
> >>> Hi Krzysztof,
> >>>
> >>> On Wed, 4 Mar 2026 09:05:11 +0100
> >>> Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>>
> >>>> On Tue, Mar 03, 2026 at 05:10:20PM +0100, Oleksij Rempel wrote:
> >>>>> Hi Krzysztof and Rob,
> >>>>>
> >>>>> On Tue, Mar 03, 2026 at 08:40:55AM -0600, Rob Herring (Arm) wrote:
> >>>>>>> .../devicetree/bindings/mfd/nxp,mc33978.yaml | 114 ++++++++++++++++++
> >>>>>>> .../bindings/pinctrl/nxp,mc33978-pinctrl.yaml | 82 +++++++++++++
> >>>>>>> 2 files changed, 196 insertions(+)
> >>>>>>> create mode 100644 Documentation/devicetree/bindings/mfd/nxp,mc33978.yaml
> >>>>>>> create mode 100644 Documentation/devicetree/bindings/pinctrl/nxp,mc33978-pinctrl.yaml
> >>>>>>>
> >>>>>>
> >>>>>> My bot found errors running 'make dt_binding_check' on your patch:
> >>>>>>
> >>>>>> yamllint warnings/errors:
> >>>>>>
> >>>>>> dtschema/dtc warnings/errors:
> >>>>>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/nxp,mc33978.example.dtb: gpio@0 (nxp,mc33978): $nodename:0: 'gpio@0' does not match '^mux-controller(@.*|-([0-9]|[1-9][0-9]+))?$'
> >>>>>> from schema $id: http://devicetree.org/schemas/mux/mux-controller.yaml
> >>>>>>
> >>>>>
> >>>>> Folding the mux node into the parent as suggested [1] causes this error.
> >>>>> Because the parent now has #mux-control-cells, the generic
> >>>>> mux-controller.yaml forces the node name to be mux-controller. Since
> >>>>> this chip is primarily a switch/GPIO controller, naming the parent SPI
> >>>>> node mux-controller@0 is misleading.
> >>>>>
> >>>>> What is the preferred way to go here?
> >>>>
> >>>> https://www.nxp.com/products/interfaces/multi-switch-detection-interface/22-i-o-msdi-programmable-current-analog-mux:MC33978
> >>>>
> >>>> Name of the mc33978 device is "programmable analog mux" and further
> >>>> description says "analog multiplexer for reading analog inputs ", so I
> >>>> don't find "mux-controller" a confusing name. It is EXACTLY a
> >>>> mux, so mux-controller.
> >>>
> >>> Sorry to chime in here. I'm afraid the NXP description on that link you posted
> >>> is a typo. It is not correct. This chip is primarily a "Switch Detection
> >>> Interface", or in other wordt a switch input interface. Wee here for the same
> >>> page for the MC34978, which is the exact same chip:
> >>>
> >>> https://www.nxp.com/products/interfaces/multi-switch-detection-interface/switch-detection-interface-22-i-os-programmable-wetting-current-temp-sensor-3-3-v-5-0-v-spi:MC34978
> >>
> >> That's MC34978 and I commented on MC33978.
> >>
> >> What is the primary function of MC33978 being described here as the base?
> >
> > The MC34978 and MC33978 are the exact same part (except for the temperature
> > range). The fact that NXP has two different web-pages with two different
> > descriptions of it certainly doesn't help, but you can also check the
> > datasheet[1] description: "MC33978: 22-channel multiple switch detection
> > interface with programmable wetting current"
> >
> > Further down in the description it says: "It also features a 24-to-1 analog
> > multiplexer for reading inputs as analog."
> > IMHO this makes it clear that this is NOT primarily a MUX.
> >
> > Actually, I doubt many users of this chip will use the analog MUX function at
> > all since it has quite a few limitations that make it not very practical to
> > use.
> >
> > The most fitting Linux framework for this chip's primary funtcion IMHO is
> > pinctrl/gpio, but there are some caveats unfortunately.
> >
> > [1] https://www.nxp.com/docs/en/data-sheet/MC33978.pdf
>
> OK, thanks for the explanation, but then primary function is not GPIO
> either, because nothing on linked page says it is a generic purpose IO.
> It says it is switch detection. Maybe better generic name is then
> "pinctrl", thus also "pinctrl" child should be folded into the parent...
> but switch detection is also not a pinctrl. :/
I agree. This chip is indeed not very clear-cut with respect to the correct
Linux subsystem. It could also be an input device if you view it strictly from
the "switches" standpoint. I thought about this also, but figured that it
would be more flexible to just view it as a pinctrl device, which could always
be used in combination with something like gpio-keys.c if one really wanted
the input functionality. For context, our use-case is primarily for industrial
control reading digital sensors such as mechanical switches or industrial
optical sensors, and that is AFAIK the main application for this chip anyway.
For this the gpio UAPI is a good match.
Best regards,
--
David Jander
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: mfd: add NXP MC33978/MC34978 MSDI
2026-03-03 16:10 ` Oleksij Rempel
2026-03-04 8:05 ` Krzysztof Kozlowski
@ 2026-03-04 18:26 ` Rob Herring
2026-03-04 18:33 ` Conor Dooley
1 sibling, 1 reply; 25+ messages in thread
From: Rob Herring @ 2026-03-04 18:26 UTC (permalink / raw)
To: Oleksij Rempel
Cc: devicetree, Krzysztof Kozlowski, Conor Dooley, Peter Rosin,
kernel, linux-kernel, linux-gpio, David Jander, Lee Jones,
Guenter Roeck, Linus Walleij, linux-hwmon
On Tue, Mar 3, 2026 at 10:10 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
>
> Hi Krzysztof and Rob,
>
> On Tue, Mar 03, 2026 at 08:40:55AM -0600, Rob Herring (Arm) wrote:
> > > .../devicetree/bindings/mfd/nxp,mc33978.yaml | 114 ++++++++++++++++++
> > > .../bindings/pinctrl/nxp,mc33978-pinctrl.yaml | 82 +++++++++++++
> > > 2 files changed, 196 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/mfd/nxp,mc33978.yaml
> > > create mode 100644 Documentation/devicetree/bindings/pinctrl/nxp,mc33978-pinctrl.yaml
> > >
> >
> > My bot found errors running 'make dt_binding_check' on your patch:
> >
> > yamllint warnings/errors:
> >
> > dtschema/dtc warnings/errors:
> > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/nxp,mc33978.example.dtb: gpio@0 (nxp,mc33978): $nodename:0: 'gpio@0' does not match '^mux-controller(@.*|-([0-9]|[1-9][0-9]+))?$'
> > from schema $id: http://devicetree.org/schemas/mux/mux-controller.yaml
> >
>
> Folding the mux node into the parent as suggested [1] causes this error.
> Because the parent now has #mux-control-cells, the generic
> mux-controller.yaml forces the node name to be mux-controller. Since
> this chip is primarily a switch/GPIO controller, naming the parent SPI
> node mux-controller@0 is misleading.
>
> What is the preferred way to go here?
I think there was another series dropping the mux-controller node
name. Not sure what happened to it, but that's what we need to do
here.
Rob
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: mfd: add NXP MC33978/MC34978 MSDI
2026-03-04 18:26 ` Rob Herring
@ 2026-03-04 18:33 ` Conor Dooley
2026-03-05 2:38 ` Rob Herring
0 siblings, 1 reply; 25+ messages in thread
From: Conor Dooley @ 2026-03-04 18:33 UTC (permalink / raw)
To: Rob Herring
Cc: Oleksij Rempel, devicetree, Krzysztof Kozlowski, Conor Dooley,
Peter Rosin, kernel, linux-kernel, linux-gpio, David Jander,
Lee Jones, Guenter Roeck, Linus Walleij, linux-hwmon
[-- Attachment #1: Type: text/plain, Size: 2018 bytes --]
On Wed, Mar 04, 2026 at 12:26:17PM -0600, Rob Herring wrote:
> On Tue, Mar 3, 2026 at 10:10 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> >
> > Hi Krzysztof and Rob,
> >
> > On Tue, Mar 03, 2026 at 08:40:55AM -0600, Rob Herring (Arm) wrote:
> > > > .../devicetree/bindings/mfd/nxp,mc33978.yaml | 114 ++++++++++++++++++
> > > > .../bindings/pinctrl/nxp,mc33978-pinctrl.yaml | 82 +++++++++++++
> > > > 2 files changed, 196 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/mfd/nxp,mc33978.yaml
> > > > create mode 100644 Documentation/devicetree/bindings/pinctrl/nxp,mc33978-pinctrl.yaml
> > > >
> > >
> > > My bot found errors running 'make dt_binding_check' on your patch:
> > >
> > > yamllint warnings/errors:
> > >
> > > dtschema/dtc warnings/errors:
> > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/nxp,mc33978.example.dtb: gpio@0 (nxp,mc33978): $nodename:0: 'gpio@0' does not match '^mux-controller(@.*|-([0-9]|[1-9][0-9]+))?$'
> > > from schema $id: http://devicetree.org/schemas/mux/mux-controller.yaml
> > >
> >
> > Folding the mux node into the parent as suggested [1] causes this error.
> > Because the parent now has #mux-control-cells, the generic
> > mux-controller.yaml forces the node name to be mux-controller. Since
> > this chip is primarily a switch/GPIO controller, naming the parent SPI
> > node mux-controller@0 is misleading.
> >
> > What is the preferred way to go here?
>
> I think there was another series dropping the mux-controller node
> name. Not sure what happened to it, but that's what we need to do
> here.
It's here:
https://lore.kernel.org/all/cover.1769703480.git.tommaso.merciai.xr@bp.renesas.com/
Tommaso pinged me about it wondering what to do. Mux maintainer has been
unresponsive. Maybe you should grab the binding portion since it is
blocking other work?
https://lore.kernel.org/all/cover.1769703480.git.tommaso.merciai.xr@bp.renesas.com/
>
> Rob
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: mfd: add NXP MC33978/MC34978 MSDI
2026-03-04 18:33 ` Conor Dooley
@ 2026-03-05 2:38 ` Rob Herring
0 siblings, 0 replies; 25+ messages in thread
From: Rob Herring @ 2026-03-05 2:38 UTC (permalink / raw)
To: Conor Dooley
Cc: Oleksij Rempel, devicetree, Krzysztof Kozlowski, Conor Dooley,
Peter Rosin, kernel, linux-kernel, linux-gpio, David Jander,
Lee Jones, Guenter Roeck, Linus Walleij, linux-hwmon
On Wed, Mar 04, 2026 at 06:33:34PM +0000, Conor Dooley wrote:
> On Wed, Mar 04, 2026 at 12:26:17PM -0600, Rob Herring wrote:
> > On Tue, Mar 3, 2026 at 10:10 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> > >
> > > Hi Krzysztof and Rob,
> > >
> > > On Tue, Mar 03, 2026 at 08:40:55AM -0600, Rob Herring (Arm) wrote:
> > > > > .../devicetree/bindings/mfd/nxp,mc33978.yaml | 114 ++++++++++++++++++
> > > > > .../bindings/pinctrl/nxp,mc33978-pinctrl.yaml | 82 +++++++++++++
> > > > > 2 files changed, 196 insertions(+)
> > > > > create mode 100644 Documentation/devicetree/bindings/mfd/nxp,mc33978.yaml
> > > > > create mode 100644 Documentation/devicetree/bindings/pinctrl/nxp,mc33978-pinctrl.yaml
> > > > >
> > > >
> > > > My bot found errors running 'make dt_binding_check' on your patch:
> > > >
> > > > yamllint warnings/errors:
> > > >
> > > > dtschema/dtc warnings/errors:
> > > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/nxp,mc33978.example.dtb: gpio@0 (nxp,mc33978): $nodename:0: 'gpio@0' does not match '^mux-controller(@.*|-([0-9]|[1-9][0-9]+))?$'
> > > > from schema $id: http://devicetree.org/schemas/mux/mux-controller.yaml
> > > >
> > >
> > > Folding the mux node into the parent as suggested [1] causes this error.
> > > Because the parent now has #mux-control-cells, the generic
> > > mux-controller.yaml forces the node name to be mux-controller. Since
> > > this chip is primarily a switch/GPIO controller, naming the parent SPI
> > > node mux-controller@0 is misleading.
> > >
> > > What is the preferred way to go here?
> >
> > I think there was another series dropping the mux-controller node
> > name. Not sure what happened to it, but that's what we need to do
> > here.
>
> It's here:
> https://lore.kernel.org/all/cover.1769703480.git.tommaso.merciai.xr@bp.renesas.com/
>
> Tommaso pinged me about it wondering what to do. Mux maintainer has been
> unresponsive. Maybe you should grab the binding portion since it is
> blocking other work?
> https://lore.kernel.org/all/cover.1769703480.git.tommaso.merciai.xr@bp.renesas.com/
It is now applied.
Rob
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: mfd: add NXP MC33978/MC34978 MSDI
2026-03-04 12:17 ` David Jander
@ 2026-03-05 12:54 ` Linus Walleij
2026-03-05 15:10 ` David Jander
0 siblings, 1 reply; 25+ messages in thread
From: Linus Walleij @ 2026-03-05 12:54 UTC (permalink / raw)
To: David Jander
Cc: Krzysztof Kozlowski, Oleksij Rempel, Rob Herring (Arm),
devicetree, Krzysztof Kozlowski, Conor Dooley, Peter Rosin,
kernel, linux-kernel, linux-gpio, Lee Jones, Guenter Roeck,
linux-hwmon
On Wed, Mar 4, 2026 at 1:18 PM David Jander <david@protonic.nl> wrote:
> > OK, thanks for the explanation, but then primary function is not GPIO
> > either, because nothing on linked page says it is a generic purpose IO.
> > It says it is switch detection. Maybe better generic name is then
> > "pinctrl", thus also "pinctrl" child should be folded into the parent...
> > but switch detection is also not a pinctrl. :/
>
> I agree. This chip is indeed not very clear-cut with respect to the correct
> Linux subsystem. It could also be an input device if you view it strictly from
> the "switches" standpoint. I thought about this also, but figured that it
> would be more flexible to just view it as a pinctrl device, which could always
> be used in combination with something like gpio-keys.c if one really wanted
> the input functionality. For context, our use-case is primarily for industrial
> control reading digital sensors such as mechanical switches or industrial
> optical sensors, and that is AFAIK the main application for this chip anyway.
> For this the gpio UAPI is a good match.
I have read the datasheets and GPIO+pinctrl is the best fit in my opinion,
mostly because there are a lot of electric properties involved and there
are industrial use cases, which is a good fit for the GPIO character
device and the pinctrl+GPIO generic pin config options, some which
are already identical.
The alternative would be to create something new in drivers/iio
which I think is overkill.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 4/6] pinctrl: add NXP MC33978/MC34978 pinctrl driver
2026-03-03 13:39 ` [PATCH v2 4/6] pinctrl: add NXP MC33978/MC34978 pinctrl driver Oleksij Rempel
@ 2026-03-05 13:41 ` Linus Walleij
0 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2026-03-05 13:41 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lee Jones, Peter Rosin, David Jander, kernel, linux-kernel,
devicetree, linux-hwmon, linux-gpio
Hi Oleksij,
this is starting to look good!
One thing bothers me:
On Tue, Mar 3, 2026 at 2:40 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> - Add a comment to mc33978_gpio_to_irq() explaining why it must act as a
> proxy to the parent MFD's irq_domain (shared physical INT_B line with hwmon).
(...)
> +static int mc33978_set(struct gpio_chip *chip, unsigned int offset, int value)
> +{
> + struct mc33978_pinctrl *mpc = gpiochip_get_data(chip);
> + int pull;
> + int ret;
> +
> + /*
> + * We emulate open-drain/-source outputs by routing or isolating the
> + * active wetting current sources.
> + * To drive the line, we apply the current source.
> + * To turn the line OFF (achieve High-Impedance), we MUST use the
> + * hardware TRI_SP / TRI_SG tri-state registers to physically isolate
> + * it.
> + */
> + if (mc33978_is_sp(offset)) {
> + pull = value ? MC33978_PU : MC33978_PD;
> + value = 1;
> + } else {
> + pull = MC33978_PU;
> + }
> +
> + mutex_lock(&mpc->lock);
Have you considered using guards for this to avoid the goto:s?
> +static int mc33978_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct mc33978_pinctrl *mpc = gpiochip_get_data(chip);
> + int virq;
> +
> + if (!mpc->domain)
> + return -ENXIO;
> +
> + /*
> + * The hardware shares a single physical INT_B line for both GPIO pin
> + * changes and internal hardware faults (hwmon). Therefore, the IRQ
> + * domain and threaded handler are centrally managed by the MFD core.
> + */
> + virq = irq_create_mapping(mpc->domain, offset);
> + if (!virq) {
> + dev_err(mpc->dev, "Failed to map hwirq %u to virq\n", offset);
> + return -ENXIO;
> + }
> +
> + return virq;
> +}
I don't know about this.
If it is clear from internal registers which bits represents the hwmon
IRQs and which bits are GPIO IRQs, the modern way to handle this
is hierarchical irqchip. Have you looked into that?
For a simple example c.f. drivers/gpio/gpio-ixp4xx.c how this
is registered with a parent irqchip using
girq->parent_domain = parent;
girq->child_to_parent_hwirq = ixp4xx_gpio_child_to_parent_hwirq;
and
ixp4xx_gpio_child_to_parent_hwirq().
This way a unique irqchip for GPIO can be created as a child
of the MFD irqchip.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: mfd: add NXP MC33978/MC34978 MSDI
2026-03-05 12:54 ` Linus Walleij
@ 2026-03-05 15:10 ` David Jander
2026-03-05 23:40 ` Linus Walleij
0 siblings, 1 reply; 25+ messages in thread
From: David Jander @ 2026-03-05 15:10 UTC (permalink / raw)
To: Linus Walleij
Cc: Krzysztof Kozlowski, Oleksij Rempel, Rob Herring (Arm),
devicetree, Krzysztof Kozlowski, Conor Dooley, Peter Rosin,
kernel, linux-kernel, linux-gpio, Lee Jones, Guenter Roeck,
linux-hwmon
On Thu, 5 Mar 2026 13:54:20 +0100
Linus Walleij <linusw@kernel.org> wrote:
> On Wed, Mar 4, 2026 at 1:18 PM David Jander <david@protonic.nl> wrote:
>
> > > OK, thanks for the explanation, but then primary function is not GPIO
> > > either, because nothing on linked page says it is a generic purpose IO.
> > > It says it is switch detection. Maybe better generic name is then
> > > "pinctrl", thus also "pinctrl" child should be folded into the parent...
> > > but switch detection is also not a pinctrl. :/
> >
> > I agree. This chip is indeed not very clear-cut with respect to the correct
> > Linux subsystem. It could also be an input device if you view it strictly from
> > the "switches" standpoint. I thought about this also, but figured that it
> > would be more flexible to just view it as a pinctrl device, which could always
> > be used in combination with something like gpio-keys.c if one really wanted
> > the input functionality. For context, our use-case is primarily for industrial
> > control reading digital sensors such as mechanical switches or industrial
> > optical sensors, and that is AFAIK the main application for this chip anyway.
> > For this the gpio UAPI is a good match.
>
> I have read the datasheets and GPIO+pinctrl is the best fit in my opinion,
> mostly because there are a lot of electric properties involved and there
> are industrial use cases, which is a good fit for the GPIO character
> device and the pinctrl+GPIO generic pin config options, some which
> are already identical.
Ack.
Sorry for this following long question to Linus. TL;DR: switch vs voltage
semantics for logical input state.
Taking the opportunity that you read the datasheet and put some thought
into this (thank you!), I'd like to know your opinion on a specific friction
point where this chip doesn't quite fit the GPIO framework exactly (or maybe
it does?):
Looking at the description of the "switch status" register (chapter 7.20.27),
it says "A Logic [1] means the switch is closed while a Logic [0] is an open
switch". This reflects the semantics of the switches being the inputs.
Nevertheless, if a switch is in SG mode (switch to ground with positive
wetting current), this means that the voltage level at the chip input pin is 0V
when the switch is closed. If the switch is in SB mode (switch to battery),
then the voltage at the chip pin is positive when the switch is closed.
In other words: the chip reports logic 1 or logic 0 in its input register
according to the state of the switch, and NOT according to the voltage at the
input pin.
The question I have is thus: Should the GPIO driver of this particular chip
report the state of the switch or should it report the state corresponding to
voltage at the input pin?
The former would mean 1:1 reporting of the "switch-state" register bits. The
latter would imply having to invert all the bits that correspond to inputs
that are in SG mode, while leaving bits corresponding to inputs in SB mode
without inverting.
I am tempted to think that hardware developers that use this chip might expect
the GPIO driver to report the state as it is read from the register. But I
suspect that the Linux kernel GPIO framework might enforce strictly the
logical state to be equal to the voltage at the pin (i.e. logic 0 == zero volt,
and logic 1 == positive non-zero voltage), but is this true?
Right now the driver inverts the "switch state" register bits accordingly if
SG mode is selected for a particular input, but maybe it is better not to do
this, since it introduces more complexity and replaces the semantics of the
chip with some (mandatory?) semantics of the Linux kernel.
Please advice.
> The alternative would be to create something new in drivers/iio
> which I think is overkill.
I agree.
OT: I think "Industrial IO" is a bit of a misnomer, as this subsystem mainly
focuses on analog IO. If we were to extend "IIO" to also digital industrial
IO, then not only would it collide with GPIO, but probably the upcoming second
iteration of the "Linux Motion Control" subsystem would also need to move
there... which is definitely overkill IMHO.
Best regards,
--
David Jander
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: mfd: add NXP MC33978/MC34978 MSDI
2026-03-05 15:10 ` David Jander
@ 2026-03-05 23:40 ` Linus Walleij
2026-03-06 8:44 ` David Jander
0 siblings, 1 reply; 25+ messages in thread
From: Linus Walleij @ 2026-03-05 23:40 UTC (permalink / raw)
To: David Jander
Cc: Krzysztof Kozlowski, Oleksij Rempel, Rob Herring (Arm),
devicetree, Krzysztof Kozlowski, Conor Dooley, Peter Rosin,
kernel, linux-kernel, linux-gpio, Lee Jones, Guenter Roeck,
linux-hwmon
On Thu, Mar 5, 2026 at 4:10 PM David Jander <david@protonic.nl> wrote:
> I am tempted to think that hardware developers that use this chip might expect
> the GPIO driver to report the state as it is read from the register. But I
> suspect that the Linux kernel GPIO framework might enforce strictly the
> logical state to be equal to the voltage at the pin (i.e. logic 0 == zero volt,
> and logic 1 == positive non-zero voltage), but is this true?
GPIO assumes all values are expressing the (raw) voltage on the pin
clamped to [0,1] logic level.
It can further invert the meaning of this using GPIO_ACTIVE_LOW,
ACTIVE LOW means the same as "overstrike" or #VAL in schematic
so if a signal is active low and low voltage on the board it is
presented as active (1) to the consumers in the kernel or
userspace.
If it represents anything else than the raw logic voltage on the line,
the semantics of GPIO_ACTIVE_LOW would be completely
confusing.
To represent switch states, I think using drivers or userspace code
should interpret this.
You can also add a custom debugfs file to your driver to help
users by providing the actual switch state and more.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: mfd: add NXP MC33978/MC34978 MSDI
2026-03-05 23:40 ` Linus Walleij
@ 2026-03-06 8:44 ` David Jander
2026-03-06 13:24 ` Linus Walleij
0 siblings, 1 reply; 25+ messages in thread
From: David Jander @ 2026-03-06 8:44 UTC (permalink / raw)
To: Linus Walleij
Cc: Krzysztof Kozlowski, Oleksij Rempel, Rob Herring (Arm),
devicetree, Krzysztof Kozlowski, Conor Dooley, Peter Rosin,
kernel, linux-kernel, linux-gpio, Lee Jones, Guenter Roeck,
linux-hwmon
On Fri, 6 Mar 2026 00:40:16 +0100
Linus Walleij <linusw@kernel.org> wrote:
> On Thu, Mar 5, 2026 at 4:10 PM David Jander <david@protonic.nl> wrote:
>
> > I am tempted to think that hardware developers that use this chip might expect
> > the GPIO driver to report the state as it is read from the register. But I
> > suspect that the Linux kernel GPIO framework might enforce strictly the
> > logical state to be equal to the voltage at the pin (i.e. logic 0 == zero volt,
> > and logic 1 == positive non-zero voltage), but is this true?
>
> GPIO assumes all values are expressing the (raw) voltage on the pin
> clamped to [0,1] logic level.
This sounds like a good, usable definition. I like it, but I can't find it in
the documentation. To make it more complete, I'd add one more clarification in
case we deal with negative voltage levels (0 and -5V for example), but it is
usable for this case. Where can I find it? Or.. should we add it to the
documentation if it isn't documented this way yet?
> It can further invert the meaning of this using GPIO_ACTIVE_LOW,
> ACTIVE LOW means the same as "overstrike" or #VAL in schematic
> so if a signal is active low and low voltage on the board it is
> presented as active (1) to the consumers in the kernel or
> userspace.
>
> If it represents anything else than the raw logic voltage on the line,
> the semantics of GPIO_ACTIVE_LOW would be completely
> confusing.
>
> To represent switch states, I think using drivers or userspace code
> should interpret this.
>
> You can also add a custom debugfs file to your driver to help
> users by providing the actual switch state and more.
Thanks for clarifying. This makes sense taking into account the above
definition of raw logic-level == raw voltage on pin clamped to [0,1].
But for the future users of this driver I would need this officially defined
somewhere, so I can point there to avoid confusion.
Best regards,
--
David Jander
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] dt-bindings: mfd: add NXP MC33978/MC34978 MSDI
2026-03-06 8:44 ` David Jander
@ 2026-03-06 13:24 ` Linus Walleij
0 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2026-03-06 13:24 UTC (permalink / raw)
To: David Jander
Cc: Krzysztof Kozlowski, Oleksij Rempel, Rob Herring (Arm),
devicetree, Krzysztof Kozlowski, Conor Dooley, Peter Rosin,
kernel, linux-kernel, linux-gpio, Lee Jones, Guenter Roeck,
linux-hwmon
On Fri, Mar 6, 2026 at 9:44 AM David Jander <david@protonic.nl> wrote:
> > GPIO assumes all values are expressing the (raw) voltage on the pin
> > clamped to [0,1] logic level.
>
> This sounds like a good, usable definition. I like it, but I can't find it in
> the documentation.
I sent a patch, thanks for noting that.
> To make it more complete, I'd add one more clarification in
> case we deal with negative voltage levels (0 and -5V for example), but it is
> usable for this case. Where can I find it? Or.. should we add it to the
> documentation if it isn't documented this way yet?
We don't really do upfront design, we deal with this case the day
we have a driver that needs it.
> Thanks for clarifying. This makes sense taking into account the above
> definition of raw logic-level == raw voltage on pin clamped to [0,1].
> But for the future users of this driver I would need this officially defined
> somewhere, so I can point there to avoid confusion.
See proposed patch, it may need some debate but it's in there.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2026-03-06 13:24 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-03 13:39 [PATCH v2 0/8] mfd: Add support for NXP MC33978/MC34978 MSDI Oleksij Rempel
2026-03-03 13:39 ` [PATCH v2 1/6] dt-bindings: mfd: add " Oleksij Rempel
2026-03-03 14:40 ` Rob Herring (Arm)
2026-03-03 16:10 ` Oleksij Rempel
2026-03-04 8:05 ` Krzysztof Kozlowski
2026-03-04 9:06 ` David Jander
2026-03-04 9:49 ` Krzysztof Kozlowski
2026-03-04 10:25 ` David Jander
2026-03-04 11:41 ` Krzysztof Kozlowski
2026-03-04 12:17 ` David Jander
2026-03-05 12:54 ` Linus Walleij
2026-03-05 15:10 ` David Jander
2026-03-05 23:40 ` Linus Walleij
2026-03-06 8:44 ` David Jander
2026-03-06 13:24 ` Linus Walleij
2026-03-04 18:26 ` Rob Herring
2026-03-04 18:33 ` Conor Dooley
2026-03-05 2:38 ` Rob Herring
2026-03-04 7:59 ` Krzysztof Kozlowski
2026-03-03 13:39 ` [PATCH v2 2/6] mfd: add NXP MC33978/MC34978 core driver Oleksij Rempel
2026-03-03 13:39 ` [PATCH v2 3/6] pinctrl: core: Make pin group callbacks optional Oleksij Rempel
2026-03-03 13:39 ` [PATCH v2 4/6] pinctrl: add NXP MC33978/MC34978 pinctrl driver Oleksij Rempel
2026-03-05 13:41 ` Linus Walleij
2026-03-03 13:39 ` [PATCH v2 5/6] hwmon: add NXP MC33978/MC34978 driver Oleksij Rempel
2026-03-03 13:39 ` [PATCH v2 6/6] mux: add NXP MC33978/MC34978 AMUX driver Oleksij Rempel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox