* [PATCH] pinctrl: mediatek: common-v1: Use a raw spinlock for the pinctrl regmap
@ 2026-09-29 17:57 Ryan Brue
2026-09-29 18:09 ` sashiko-bot
2026-09-30 8:06 ` Sebastian Andrzej Siewior
0 siblings, 2 replies; 4+ messages in thread
From: Ryan Brue @ 2026-09-29 17:57 UTC (permalink / raw)
To: Sean Wang, Linus Walleij, Matthias Brugger,
AngeloGioacchino Del Regno, Bartosz Golaszewski,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Yingjoe Chen, Chaotian Jing, Hongzhou Yang
Cc: linux-mediatek, linux-gpio, linux-kernel, linux-arm-kernel,
linux-rt-devel, Ryan Brue
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.
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@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 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,
--
Ryan Brue <ryanbrue.dev@gmail.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] pinctrl: mediatek: common-v1: Use a raw spinlock for the pinctrl regmap
2026-09-29 17:57 [PATCH] pinctrl: mediatek: common-v1: Use a raw spinlock for the pinctrl regmap Ryan Brue
@ 2026-09-29 18:09 ` sashiko-bot
2026-09-30 8:06 ` Sebastian Andrzej Siewior
1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-09-29 18:09 UTC (permalink / raw)
To: Ryan Brue; +Cc: linux-kernel, linux-rt-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] pinctrl: mediatek: fallback to syscon bypasses lockdep fix
- [Medium] pinctrl: mediatek: integer underflow in max_register calculation
--
commit b8dcd543528b096e44b07a9f7d47936a1a0bdd08
Author: Ryan Brue <ryanbrue.dev@gmail.com>
pinctrl: mediatek: common-v1: Use a raw spinlock for the pinctrl regmap
This commit introduces a new regmap with a raw spinlock for Mediatek
pinctrl nodes to resolve invalid wait context splats in hard IRQ context.
It registers this regmap with syscon so other node users can share it,
and falls back to an existing syscon regmap if one is already registered.
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> index 1a977acd6883a..65b1e3096183d 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
[ ... ]
> +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;
[Severity: Medium]
Can this calculation underflow if the mapped resource size in the device
tree is smaller than config.reg_stride?
Because resource_size() returns an unsigned resource_size_t, a small
resource size (e.g., 1, 2, or 3) would underflow and set max_register to
an extremely large value. This seems to bypass regmap memory bounds checking.
The standard syscon code in drivers/mfd/syscon.c usually checks if the
resource size is smaller than the IO width to prevent this.
> + 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);
[Severity: Medium]
Does this fallback path silently inherit a non-raw spinlock regmap if another
driver (like ethernet) already triggered its creation?
If another driver sharing the syscon node probes before this pinctrl driver,
the standard syscon initialization will create a regmap with a normal,
sleepable spinlock_t. When the pinctrl driver subsequently probes,
of_syscon_register_regmap() returns -EEXIST, and we fall back to
syscon_node_to_regmap().
Would this result in the pinctrl driver using the existing non-raw
spinlock regmap, bypassing the intended lockdep fix and potentially causing
lockdep warnings on PREEMPT_RT when accessed from hard IRQ context?
> + }
> +
> + return regmap;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260929-rbrue-suez-upstreaming-mtk-pinctrl-raw-regmap-v1-1-db92943f42cb@gmail.com?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] pinctrl: mediatek: common-v1: Use a raw spinlock for the pinctrl regmap
2026-09-29 17:57 [PATCH] pinctrl: mediatek: common-v1: Use a raw spinlock for the pinctrl regmap Ryan Brue
2026-09-29 18:09 ` sashiko-bot
@ 2026-09-30 8:06 ` Sebastian Andrzej Siewior
2026-09-30 8:17 ` Chen-Yu Tsai
1 sibling, 1 reply; 4+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-09-30 8:06 UTC (permalink / raw)
To: Ryan Brue, Mark Brown, Lee Jones, Arnd Bergmann
Cc: Sean Wang, Linus Walleij, Matthias Brugger,
AngeloGioacchino Del Regno, Bartosz Golaszewski, Clark Williams,
Steven Rostedt, Yingjoe Chen, Chaotian Jing, Hongzhou Yang,
linux-mediatek, linux-gpio, linux-kernel, linux-arm-kernel,
linux-rt-devel, mfd
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.
> 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@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 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] pinctrl: mediatek: common-v1: Use a raw spinlock for the pinctrl regmap
2026-09-30 8:06 ` Sebastian Andrzej Siewior
@ 2026-09-30 8:17 ` Chen-Yu Tsai
0 siblings, 0 replies; 4+ messages in thread
From: Chen-Yu Tsai @ 2026-09-30 8:17 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, Ryan Brue
Cc: Mark Brown, Lee Jones, Arnd Bergmann, Sean Wang, Linus Walleij,
Matthias Brugger, AngeloGioacchino Del Regno,
Bartosz Golaszewski, Clark Williams, Steven Rostedt,
Yingjoe Chen, Chaotian Jing, Hongzhou Yang, linux-mediatek,
linux-gpio, linux-kernel, linux-arm-kernel, linux-rt-devel, mfd
On Wed, Sep 30, 2026 at 4:06 PM Sebastian Andrzej Siewior
<bigeasy@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@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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-09-30 8:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-29 17:57 [PATCH] pinctrl: mediatek: common-v1: Use a raw spinlock for the pinctrl regmap Ryan Brue
2026-09-29 18:09 ` sashiko-bot
2026-09-30 8:06 ` Sebastian Andrzej Siewior
2026-09-30 8:17 ` Chen-Yu Tsai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®