[PATCH] pinctrl: mediatek: common-v1: Use a raw spinlock for the pinctrl regmap
Chen-Yu Tsai
wenst at chromium.org
Wed Sep 30 01:17:31 PDT 2026
On Wed, Sep 30, 2026 at 4:06 PM Sebastian Andrzej Siewior
<bigeasy at linutronix.de> wrote:
>
> On 2026-09-29 12:57:51 [-0500], Ryan Brue wrote:
> > The EINT irq_chip emulates both-edge interrupts by reading the pin's
> > level through mtk_gpio_get() and the pinctrl regmap. It does so from its
> > unmask and set_type callbacks, under the raw irq_desc lock, and from the
> > chained handler, in hard interrupt context. The regmap comes from syscon
> > and locks with a spinlock_t, which may sleep on PREEMPT_RT. With
> > CONFIG_PROVE_LOCKING, the first request of a both-edge EINT prints
> > "[ BUG: Invalid wait context ]" and turns lockdep off for the rest of
> > the boot.
>
> This duplicates syscon node and creates a new one with the
> .use_raw_spinlock=true attribute. Now, syscon is always low-level access
> with MMIO access, right?
>
> I've been wondering if we could make drivers/mfd/syscon.c use the
> raw_spintlock_t instead making this sort of change for every driver that
> has this "requirement".
>
> If this is all MMIO reads/ writes then it should work. I'm not sure why
> we have the lock to begin with. Probably due to the cache/ async writes.
> Cache wise just the flat-cache works since the other (like rbtree)
> allocates memory on write under the lock. So this does not work.
> What I am bit worried about are the bulk_write and multi_reg_write where
> multiple writes happen under the lock.
AFAIK the lock is primarily there to serialize concurrent MMIO access,
especially read-modify-write patterns in regmap_*_bits().
> > Create the regmap for the "mediatek,pctl-regmap" nodes here instead,
> > with use_raw_spinlock set, and register it with syscon so that other
> > users of a node, such as the ethernet on MT2701 and MT7623, share its
> > lock. If the node already has a syscon regmap, keep using it. Select
> > REGMAP_MMIO, which the driver now uses directly.
> >
> > Fixes: 3221f40b7631 ("pinctrl: mediatek: emulate GPIO interrupt on both-edges")
> > Assisted-by: LLM
> > Signed-off-by: Ryan Brue <ryanbrue.dev at gmail.com>
> > ---
> > Found on the Amazon Fire HD 10 (2017), an MT8173 tablet that is not
> > upstream yet, where usb_extcon_probe() requests the USB ID pin's
> > both-edge EINT. With this patch lockdep stays on through boot, CPU
> > hotplug, suspend to RAM, and lid open/close edges on the hall sensor's
> > both-edge EINT. Only MT8173 was tested. Nothing else uses the node
> > there, so the -EEXIST fallback was not exercised.
> >
> > checkpatch warns that the regmap_config should be const. It is copied
> > per node to set name and max_register, as syscon does.
> > ---
> > drivers/pinctrl/mediatek/Kconfig | 1 +
> > drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 59 ++++++++++++++++++++++++++-
> > 2 files changed, 58 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pinctrl/mediatek/Kconfig b/drivers/pinctrl/mediatek/Kconfig
> > index 30ef3dc5dfb1..764256901d6a 100644
> > --- a/drivers/pinctrl/mediatek/Kconfig
> > +++ b/drivers/pinctrl/mediatek/Kconfig
> > @@ -17,6 +17,7 @@ config PINCTRL_MTK
> > select GENERIC_PINCONF
> > select GPIOLIB
> > select EINT_MTK
> > + select REGMAP_MMIO
> >
> > config PINCTRL_MTK_V2
> > tristate
> > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> > index 1a977acd6883..65b1e3096183 100644
> > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> > @@ -9,6 +9,7 @@
> > #include <linux/gpio/driver.h>
> > #include <linux/module.h>
> > #include <linux/of.h>
> > +#include <linux/of_address.h>
> > #include <linux/of_irq.h>
> > #include <linux/pinctrl/consumer.h>
> > #include <linux/pinctrl/machine.h>
> > @@ -1057,6 +1058,60 @@ static int mtk_eint_init(struct mtk_pinctrl *pctl, struct platform_device *pdev)
> > return mtk_eint_do_init(pctl->eint, NULL);
> > }
> >
> > +static const struct regmap_config mtk_pctrl_regmap_config = {
> > + .reg_bits = 32,
> > + .val_bits = 32,
> > + .reg_stride = 4,
> > + .use_raw_spinlock = true,
> > +};
> > +
> > +/*
> > + * The EINT irq_chip reads a pin's level through this regmap from callbacks
> > + * that run under the raw irq_desc lock, so the regmap has to use a raw
> > + * spinlock too, which syscon's own does not. Register one with syscon for the
> > + * node instead, so that any other user of the node shares its lock.
> > + */
> > +static struct regmap *mtk_pctrl_syscon_regmap(struct device_node *np)
> > +{
> > + struct regmap_config config = mtk_pctrl_regmap_config;
> > + struct regmap *regmap;
> > + struct resource res;
> > + void __iomem *base;
> > + int ret;
> > +
> > + ret = of_address_to_resource(np, 0, &res);
> > + if (ret)
> > + return ERR_PTR(ret);
> > +
> > + base = ioremap(res.start, resource_size(&res));
> > + if (!base)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + config.name = kasprintf(GFP_KERNEL, "%pOFn@%pa", np, &res.start);
> > + if (!config.name) {
> > + iounmap(base);
> > + return ERR_PTR(-ENOMEM);
> > + }
> > +
> > + config.max_register = resource_size(&res) - config.reg_stride;
> > + regmap = regmap_init_mmio(NULL, base, &config);
> > + kfree(config.name);
> > + if (IS_ERR(regmap)) {
> > + iounmap(base);
> > + return regmap;
> > + }
> > +
> > + ret = of_syscon_register_regmap(np, regmap);
> > + if (ret) {
> > + regmap_exit(regmap);
> > + iounmap(base);
> > + /* An earlier probe, or another user of the node, got there first. */
> > + return ret == -EEXIST ? syscon_node_to_regmap(np) : ERR_PTR(ret);
> > + }
> > +
> > + return regmap;
> > +}
This defeats the purpose of having a phandle to a syscon node. Now you
have two regmaps that don't share locking, so both could end up touching
the same register in a read-modify-write operation and overwrite one or
the other.
The syscon node is the provider of the regmap. You need to fix it there,
not duplicate it in the consumer.
ChenYu
> > +
> > /* This is used as a common probe function */
> > int mtk_pctrl_init(struct platform_device *pdev,
> > const struct mtk_pinctrl_devdata *data,
> > @@ -1076,7 +1131,7 @@ int mtk_pctrl_init(struct platform_device *pdev,
> >
> > node = of_parse_phandle(np, "mediatek,pctl-regmap", 0);
> > if (node) {
> > - pctl->regmap1 = syscon_node_to_regmap(node);
> > + pctl->regmap1 = mtk_pctrl_syscon_regmap(node);
> > of_node_put(node);
> > if (IS_ERR(pctl->regmap1))
> > return PTR_ERR(pctl->regmap1);
> > @@ -1089,7 +1144,7 @@ int mtk_pctrl_init(struct platform_device *pdev,
> > /* Only 8135 has two base addr, other SoCs have only one. */
> > node = of_parse_phandle(np, "mediatek,pctl-regmap", 1);
> > if (node) {
> > - pctl->regmap2 = syscon_node_to_regmap(node);
> > + pctl->regmap2 = mtk_pctrl_syscon_regmap(node);
> > of_node_put(node);
> > if (IS_ERR(pctl->regmap2))
> > return PTR_ERR(pctl->regmap2);
> >
> > ---
> > base-commit: 72d3fcf802c45d00b300f25b848a93c3a2bd7c7e
> > change-id: 20260925-rbrue-suez-upstreaming-mtk-pinctrl-raw-regmap-154d6225b977
> >
> > Best regards,
>
> Sebastian
>
More information about the Linux-mediatek
mailing list