pinctrl: add lock in mtk_rmw function.

Message ID 20210312063502.3685-2-zhiyong.tao@mediatek.com
State New
Headers show
Series
  • pinctrl: add lock in mtk_rmw function.
Related show

Commit Message

zhiyong tao March 12, 2021, 6:35 a.m.
When multiple threads operate on the same register resource
which include multiple pin, It will make the register resource
wrong to control. So we add lock to avoid the case.

Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com>
---
 drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c | 4 ++++
 drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h | 2 ++
 drivers/pinctrl/mediatek/pinctrl-paris.c         | 2 ++
 3 files changed, 8 insertions(+)

Comments

Sean Wang March 16, 2021, 5:05 a.m. | #1
Hi Zhiyong,

On Fri, Mar 12, 2021 at 2:35 PM Zhiyong Tao <zhiyong.tao@mediatek.com> wrote:
>
> When multiple threads operate on the same register resource
> which include multiple pin, It will make the register resource
> wrong to control. So we add lock to avoid the case.
>
> Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com>
> ---
>  drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c | 4 ++++
>  drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h | 2 ++
>  drivers/pinctrl/mediatek/pinctrl-paris.c         | 2 ++
>  3 files changed, 8 insertions(+)
>
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> index 72f17f26acd8..fcf7c3eeee4a 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> @@ -58,10 +58,14 @@ void mtk_rmw(struct mtk_pinctrl *pctl, u8 i, u32 reg, u32 mask, u32 set)
>  {
>         u32 val;
>
> +       mutex_lock(&pctl->lock);
> +
>         val = mtk_r32(pctl, i, reg);
>         val &= ~mask;
>         val |= set;
>         mtk_w32(pctl, i, reg, val);
> +
> +       mutex_unlock(&pctl->lock);
>  }
>
>  static int mtk_hw_pin_field_lookup(struct mtk_pinctrl *hw,
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h
> index e2aae285b5fc..65eac708a3b3 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h
> +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h
> @@ -251,6 +251,8 @@ struct mtk_pinctrl {
>         struct mtk_eint                 *eint;
>         struct mtk_pinctrl_group        *groups;
>         const char          **grp_names;
> +       /* lock pin's register resource to avoid multiple threads issue*/
> +       struct mutex lock;
>  };
>
>  void mtk_rmw(struct mtk_pinctrl *pctl, u8 i, u32 reg, u32 mask, u32 set);
> diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/pinctrl-paris.c
> index da1f19288aa6..48e823f6d293 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-paris.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-paris.c
> @@ -970,6 +970,8 @@ int mtk_paris_pinctrl_probe(struct platform_device *pdev,
>
>         hw->nbase = hw->soc->nbase_names;
>
> +       mutex_init(&hw->lock);
> +

Could you help add the mutex initialization into pinctrl-moore.c too?
and then the patch would look good to me.

>         err = mtk_pctrl_build_state(pdev);
>         if (err) {
>                 dev_err(&pdev->dev, "build state failed: %d\n", err);
> --
> 2.18.0
>
zhiyong tao March 16, 2021, 6:10 a.m. | #2
On Tue, 2021-03-16 at 13:05 +0800, Sean Wang wrote:
> Hi Zhiyong,

> 

> On Fri, Mar 12, 2021 at 2:35 PM Zhiyong Tao <zhiyong.tao@mediatek.com> wrote:

> >

> > When multiple threads operate on the same register resource

> > which include multiple pin, It will make the register resource

> > wrong to control. So we add lock to avoid the case.

> >

> > Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com>

> > ---

> >  drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c | 4 ++++

> >  drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h | 2 ++

> >  drivers/pinctrl/mediatek/pinctrl-paris.c         | 2 ++

> >  3 files changed, 8 insertions(+)

> >

> > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c

> > index 72f17f26acd8..fcf7c3eeee4a 100644

> > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c

> > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c

> > @@ -58,10 +58,14 @@ void mtk_rmw(struct mtk_pinctrl *pctl, u8 i, u32 reg, u32 mask, u32 set)

> >  {

> >         u32 val;

> >

> > +       mutex_lock(&pctl->lock);

> > +

> >         val = mtk_r32(pctl, i, reg);

> >         val &= ~mask;

> >         val |= set;

> >         mtk_w32(pctl, i, reg, val);

> > +

> > +       mutex_unlock(&pctl->lock);

> >  }

> >

> >  static int mtk_hw_pin_field_lookup(struct mtk_pinctrl *hw,

> > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h

> > index e2aae285b5fc..65eac708a3b3 100644

> > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h

> > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h

> > @@ -251,6 +251,8 @@ struct mtk_pinctrl {

> >         struct mtk_eint                 *eint;

> >         struct mtk_pinctrl_group        *groups;

> >         const char          **grp_names;

> > +       /* lock pin's register resource to avoid multiple threads issue*/

> > +       struct mutex lock;

> >  };

> >

> >  void mtk_rmw(struct mtk_pinctrl *pctl, u8 i, u32 reg, u32 mask, u32 set);

> > diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/pinctrl-paris.c

> > index da1f19288aa6..48e823f6d293 100644

> > --- a/drivers/pinctrl/mediatek/pinctrl-paris.c

> > +++ b/drivers/pinctrl/mediatek/pinctrl-paris.c

> > @@ -970,6 +970,8 @@ int mtk_paris_pinctrl_probe(struct platform_device *pdev,

> >

> >         hw->nbase = hw->soc->nbase_names;

> >

> > +       mutex_init(&hw->lock);

> > +

> 

> Could you help add the mutex initialization into pinctrl-moore.c too?

> and then the patch would look good to me.


==>Hi sean,
We will add add the mutex initialization into pinctrl-moore.c too in v2.

Thanks.
> 

> >         err = mtk_pctrl_build_state(pdev);

> >         if (err) {

> >                 dev_err(&pdev->dev, "build state failed: %d\n", err);

> > --

> > 2.18.0

> >

Patch

diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
index 72f17f26acd8..fcf7c3eeee4a 100644
--- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
+++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
@@ -58,10 +58,14 @@  void mtk_rmw(struct mtk_pinctrl *pctl, u8 i, u32 reg, u32 mask, u32 set)
 {
 	u32 val;
 
+	mutex_lock(&pctl->lock);
+
 	val = mtk_r32(pctl, i, reg);
 	val &= ~mask;
 	val |= set;
 	mtk_w32(pctl, i, reg, val);
+
+	mutex_unlock(&pctl->lock);
 }
 
 static int mtk_hw_pin_field_lookup(struct mtk_pinctrl *hw,
diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h
index e2aae285b5fc..65eac708a3b3 100644
--- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h
+++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.h
@@ -251,6 +251,8 @@  struct mtk_pinctrl {
 	struct mtk_eint			*eint;
 	struct mtk_pinctrl_group	*groups;
 	const char          **grp_names;
+	/* lock pin's register resource to avoid multiple threads issue*/
+	struct mutex lock;
 };
 
 void mtk_rmw(struct mtk_pinctrl *pctl, u8 i, u32 reg, u32 mask, u32 set);
diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/pinctrl-paris.c
index da1f19288aa6..48e823f6d293 100644
--- a/drivers/pinctrl/mediatek/pinctrl-paris.c
+++ b/drivers/pinctrl/mediatek/pinctrl-paris.c
@@ -970,6 +970,8 @@  int mtk_paris_pinctrl_probe(struct platform_device *pdev,
 
 	hw->nbase = hw->soc->nbase_names;
 
+	mutex_init(&hw->lock);
+
 	err = mtk_pctrl_build_state(pdev);
 	if (err) {
 		dev_err(&pdev->dev, "build state failed: %d\n", err);