Currently pinctrl_select_state is an export symbol and don't have effective re-entrance protect design. And possible of pinctrl state changed during pinctrl_commit_state handling. Add per pinctrl lock to ensure the old state and new state transition atomization. Move dev error print message right before old_state pinctrl_select_state and out of lock protection to avoid console related driver call pinctrl_select_state recursively. Fixes: 4198a9b57106 ("pinctrl: avoid reload of p state in list iteration") Signed-off-by: Maria Yu <quic_aiquny@xxxxxxxxxxx> --- drivers/pinctrl/core.c | 11 +++++++++-- drivers/pinctrl/core.h | 2 ++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c index f2977eb65522..a19c286bf82e 100644 --- a/drivers/pinctrl/core.c +++ b/drivers/pinctrl/core.c @@ -1066,6 +1066,7 @@ static struct pinctrl *create_pinctrl(struct device *dev, p->dev = dev; INIT_LIST_HEAD(&p->states); INIT_LIST_HEAD(&p->dt_maps); + spin_lock_init(&p->lock); ret = pinctrl_dt_to_map(p, pctldev); if (ret < 0) { @@ -1262,9 +1263,12 @@ static void pinctrl_link_add(struct pinctrl_dev *pctldev, static int pinctrl_commit_state(struct pinctrl *p, struct pinctrl_state *state) { struct pinctrl_setting *setting, *setting2; - struct pinctrl_state *old_state = READ_ONCE(p->state); + struct pinctrl_state *old_state; int ret; + unsigned long flags; + spin_lock_irqsave(&p->lock, flags); + old_state = p->state; if (old_state) { /* * For each pinmux setting in the old state, forget SW's record @@ -1329,11 +1333,11 @@ static int pinctrl_commit_state(struct pinctrl *p, struct pinctrl_state *state) } p->state = state; + spin_unlock_irqrestore(&p->lock, flags); return 0; unapply_new_state: - dev_err(p->dev, "Error applying setting, reverse things back\n"); list_for_each_entry(setting2, &state->settings, node) { if (&setting2->node == &setting->node) @@ -1349,6 +1353,9 @@ static int pinctrl_commit_state(struct pinctrl *p, struct pinctrl_state *state) pinmux_disable_setting(setting2); } + spin_unlock_irqrestore(&p->lock, flags); + + dev_err(p->dev, "Error applying setting, reverse things back\n"); /* There's no infinite recursive loop here because p->state is NULL */ if (old_state) pinctrl_select_state(p, old_state); diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h index 530370443c19..86fc41393f7b 100644 --- a/drivers/pinctrl/core.h +++ b/drivers/pinctrl/core.h @@ -12,6 +12,7 @@ #include <linux/list.h> #include <linux/mutex.h> #include <linux/radix-tree.h> +#include <linux/spinlock.h> #include <linux/types.h> #include <linux/pinctrl/machine.h> @@ -91,6 +92,7 @@ struct pinctrl { struct pinctrl_state *state; struct list_head dt_maps; struct kref users; + spinlock_t lock; }; /** base-commit: 994d5c58e50e91bb02c7be4a91d5186292a895c8 -- 2.17.1