Thread (20 messages) 20 messages, 3 authors, 2026-02-27

Re: [PATCH v1 4/8] pinctrl: add NXP MC33978/MC34978 pinctrl driver

From: Oleksij Rempel <o.rempel@pengutronix.de>
Date: 2026-02-27 10:58:39
Also in: linux-gpio, linux-hwmon, lkml

Hi Linus,

On Fri, Feb 27, 2026 at 12:40:40AM +0100, Linus Walleij wrote:
..
quoted
+static int mc33978_get(struct gpio_chip *chip, unsigned int offset)
+{
+       struct mc33978_pinctrl *mpc = gpiochip_get_data(chip);
+       int status, ret;
+       bool is_switch_closed;
+       bool is_switch_to_ground = true; /* Default for all SG pins */
+
+       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);
+
+       /* Translate hardware switch semantics to logical GPIO levels */
+       if (is_switch_to_ground) {
+               /* SG: Switch open -> High (1), Switch to GND -> Low (0) */
+               status = !is_switch_closed;
+       } else {
+               /* SB: Switch open -> Low (0), Switch to Vbat -> High (1) */
+               status = is_switch_closed;
+       }
I don't think this is right.

The driver needs to report the *physical* level on the line. Then the
lines need to be flagged with GPIO_ACTIVE_LOW or GPIO_ACTIVE_HIGH
on the consumers in the device tree.
Returning the physical level is actually exactly what this code is
trying to do. I need to rewrite the comment :) 

The issue is that the MC33978 hardware does not report the physical
voltage level on the pin. As per section 9.10.27 (Read switch status) of
the datasheet: "A Logic [1] means the switch is closed while a Logic [0]
is an open switch."

Because it only reports this abstract "contact status", I have to
translate it back to the actual physical voltage level (1 = High, 0 =
Low) based on the pin's current configuration:

In Switch-to-Ground (SG) mode: the status bit stays 0 when the physical
voltage on the line is High (open), and reports 1 when the physical
voltage is Low (shorted to ground).
In Switch-to-Battery (SB) mode: the exact opposite happens.

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 |
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help