linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] eint: add gpio vritual number select
       [not found] <1544693783-25079-1-git-send-email-chuanjia.liu@mediatek.com>
@ 2018-12-13 19:33 ` Sean Wang
  2018-12-17  3:15   ` Chuanjia Liu
  2018-12-13 19:51 ` Sean Wang
  1 sibling, 1 reply; 8+ messages in thread
From: Sean Wang @ 2018-12-13 19:33 UTC (permalink / raw)
  To: chuanjia.liu
  Cc: Linus Walleij, Matthias Brugger, linux-mediatek,
	linux-arm-kernel, linux-kernel, hongkun.cao, youlin.pei,
	eddie.huang, zhiyong.tao, hailong.fan

On Thu, Dec 13, 2018 at 1:36 AM <chuanjia.liu@mediatek.com> wrote:
>
> From: Chuanjia Liu <Chuanjia.Liu@mediatek.com>
>
> This patch add gpio vritual number select,avoid virtual gpio set SMT.

s/gpio/GPIO/
s/vritual/virtual/

Virtual GPIOs you said here that means these pins only used inside SoC
and not being exported to outside SoC, right? It seems this kind of
pins doesn't need SMT.

>
> Signed-off-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com>
> ---
>  drivers/pinctrl/mediatek/mtk-eint.h              |    1 +
>  drivers/pinctrl/mediatek/pinctrl-mt8183.c        |    1 +
>  drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c |    9 ++++++---
>  3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pinctrl/mediatek/mtk-eint.h b/drivers/pinctrl/mediatek/mtk-eint.h
> index 48468d0..c16beaf 100644
> --- a/drivers/pinctrl/mediatek/mtk-eint.h
> +++ b/drivers/pinctrl/mediatek/mtk-eint.h
> @@ -37,6 +37,7 @@ struct mtk_eint_hw {
>         u8              ports;
>         unsigned int    ap_num;
>         unsigned int    db_cnt;
> +       unsigned int    vir_start;
>  };
>
>  struct mtk_eint;
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8183.c b/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> index 6262fd3..bbeafd3 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> @@ -497,6 +497,7 @@
>         .ports     = 6,
>         .ap_num    = 212,
>         .db_cnt    = 13,
> +       .vir_start = 180,
>  };
>
>  static const struct mtk_pin_soc mt8183_data = {
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> index 4a9e0d4..ca3bae1 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> @@ -289,9 +289,12 @@ static int mtk_xt_set_gpio_as_eint(void *data, unsigned long eint_n)
>         if (err)
>                 return err;
>
> -       err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT, MTK_ENABLE);
> -       if (err)
> -               return err;
> +       if (gpio_n < hw->eint->hw->vir_start) {
> +               err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
> +                                      MTK_ENABLE);
> +               if (err)
> +                       return err;
> +       }

The changes will break these SoCs without a properly configured vir_start.

If SMT seems unnecessary for these kinds of virtual GPIOs pin in the
path, we can do it as

err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
                                        MTK_ENABLE);
/* please add comments for the exclusion condition */
if (err && err != -ENOTSUPP)
        return err;

If there is getting much special on certain pins between SoCs, and
then we can consider creating a desc->flag to split logic.

>
>         return 0;
>  }
> --
> 1.7.9.5

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] eint: add gpio vritual number select
       [not found] <1544693783-25079-1-git-send-email-chuanjia.liu@mediatek.com>
  2018-12-13 19:33 ` [PATCH] eint: add gpio vritual number select Sean Wang
@ 2018-12-13 19:51 ` Sean Wang
  2018-12-17  3:19   ` Chuanjia Liu
  1 sibling, 1 reply; 8+ messages in thread
From: Sean Wang @ 2018-12-13 19:51 UTC (permalink / raw)
  To: chuanjia.liu
  Cc: Linus Walleij, Matthias Brugger, linux-mediatek,
	linux-arm-kernel, linux-kernel, hongkun.cao, youlin.pei,
	eddie.huang, zhiyong.tao, hailong.fan

And the subject should be also corrected with prefix starting with
"pinctrl: mediatek:", typo fixup, and having a better subject close to
the content.

On Thu, Dec 13, 2018 at 1:36 AM <chuanjia.liu@mediatek.com> wrote:
>
> From: Chuanjia Liu <Chuanjia.Liu@mediatek.com>
>
> This patch add gpio vritual number select,avoid virtual gpio set SMT.
>
> Signed-off-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com>
> ---
>  drivers/pinctrl/mediatek/mtk-eint.h              |    1 +
>  drivers/pinctrl/mediatek/pinctrl-mt8183.c        |    1 +
>  drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c |    9 ++++++---
>  3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pinctrl/mediatek/mtk-eint.h b/drivers/pinctrl/mediatek/mtk-eint.h
> index 48468d0..c16beaf 100644
> --- a/drivers/pinctrl/mediatek/mtk-eint.h
> +++ b/drivers/pinctrl/mediatek/mtk-eint.h
> @@ -37,6 +37,7 @@ struct mtk_eint_hw {
>         u8              ports;
>         unsigned int    ap_num;
>         unsigned int    db_cnt;
> +       unsigned int    vir_start;
>  };
>
>  struct mtk_eint;
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8183.c b/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> index 6262fd3..bbeafd3 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> @@ -497,6 +497,7 @@
>         .ports     = 6,
>         .ap_num    = 212,
>         .db_cnt    = 13,
> +       .vir_start = 180,
>  };
>
>  static const struct mtk_pin_soc mt8183_data = {
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> index 4a9e0d4..ca3bae1 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> @@ -289,9 +289,12 @@ static int mtk_xt_set_gpio_as_eint(void *data, unsigned long eint_n)
>         if (err)
>                 return err;
>
> -       err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT, MTK_ENABLE);
> -       if (err)
> -               return err;
> +       if (gpio_n < hw->eint->hw->vir_start) {
> +               err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
> +                                      MTK_ENABLE);
> +               if (err)
> +                       return err;
> +       }
>
>         return 0;
>  }
> --
> 1.7.9.5

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] eint: add gpio vritual number select
  2018-12-13 19:33 ` [PATCH] eint: add gpio vritual number select Sean Wang
@ 2018-12-17  3:15   ` Chuanjia Liu
  2018-12-17  4:04     ` Yingjoe Chen
  2018-12-17  7:59     ` Sean Wang
  0 siblings, 2 replies; 8+ messages in thread
From: Chuanjia Liu @ 2018-12-17  3:15 UTC (permalink / raw)
  To: Sean Wang
  Cc: Linus Walleij, Matthias Brugger, linux-mediatek,
	linux-arm-kernel, linux-kernel, hongkun.cao, youlin.pei,
	eddie.huang, zhiyong.tao, hailong.fan

On Thu, 2018-12-13 at 11:33 -0800, Sean Wang wrote:
> On Thu, Dec 13, 2018 at 1:36 AM <chuanjia.liu@mediatek.com> wrote:
> >
> > From: Chuanjia Liu <Chuanjia.Liu@mediatek.com>
> >
> > This patch add gpio vritual number select,avoid virtual gpio set SMT.
> 
> s/gpio/GPIO/
> s/vritual/virtual/
> 
> Virtual GPIOs you said here that means these pins only used inside SoC
> and not being exported to outside SoC, right? It seems this kind of
> pins doesn't need SMT.
> 
Yes,virtual gpio only used inside SOC and these pins doesn't need set
SMT
> >
> > Signed-off-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com>
> > ---
> >  drivers/pinctrl/mediatek/mtk-eint.h              |    1 +
> >  drivers/pinctrl/mediatek/pinctrl-mt8183.c        |    1 +
> >  drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c |    9 ++++++---
> >  3 files changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pinctrl/mediatek/mtk-eint.h b/drivers/pinctrl/mediatek/mtk-eint.h
> > index 48468d0..c16beaf 100644
> > --- a/drivers/pinctrl/mediatek/mtk-eint.h
> > +++ b/drivers/pinctrl/mediatek/mtk-eint.h
> > @@ -37,6 +37,7 @@ struct mtk_eint_hw {
> >         u8              ports;
> >         unsigned int    ap_num;
> >         unsigned int    db_cnt;
> > +       unsigned int    vir_start;
> >  };
> >
> >  struct mtk_eint;
> > diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8183.c b/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> > index 6262fd3..bbeafd3 100644
> > --- a/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> > +++ b/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> > @@ -497,6 +497,7 @@
> >         .ports     = 6,
> >         .ap_num    = 212,
> >         .db_cnt    = 13,
> > +       .vir_start = 180,
> >  };
> >
> >  static const struct mtk_pin_soc mt8183_data = {
> > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> > index 4a9e0d4..ca3bae1 100644
> > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> > @@ -289,9 +289,12 @@ static int mtk_xt_set_gpio_as_eint(void *data, unsigned long eint_n)
> >         if (err)
> >                 return err;
> >
> > -       err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT, MTK_ENABLE);
> > -       if (err)
> > -               return err;
> > +       if (gpio_n < hw->eint->hw->vir_start) {
> > +               err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
> > +                                      MTK_ENABLE);
> > +               if (err)
> > +                       return err;
> > +       }
> 
> The changes will break these SoCs without a properly configured vir_start.
> 
> If SMT seems unnecessary for these kinds of virtual GPIOs pin in the
> path, we can do it as
> 
> err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
>                                         MTK_ENABLE);
> /* please add comments for the exclusion condition */
> if (err && err != -ENOTSUPP)
>         return err;
> 
> If there is getting much special on certain pins between SoCs, and
> then we can consider creating a desc->flag to split logic.

Yes,SMT unnecessary for these kinds of virtual GPIOS pin in the path,if
do it as
	err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
                                        MTK_ENABLE);
	if (err && err != -ENOTSUPP)
      		  return err;
I wonder if system will lose -ENOTSUPP information when smt was not
successfully set by real gpio?
> 
> >
> >         return 0;
> >  }
> > --
> > 1.7.9.5



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] eint: add gpio vritual number select
  2018-12-13 19:51 ` Sean Wang
@ 2018-12-17  3:19   ` Chuanjia Liu
  0 siblings, 0 replies; 8+ messages in thread
From: Chuanjia Liu @ 2018-12-17  3:19 UTC (permalink / raw)
  To: Sean Wang
  Cc: Linus Walleij, Matthias Brugger, linux-mediatek,
	linux-arm-kernel, linux-kernel,
	Hongkun Cao (曹洪坤),
	Youlin Pei (裴友林),
	Eddie Huang (黃智傑),
	Zhiyong Tao (陶志勇),
	Hailong Fan (范海龙)

On Fri, 2018-12-14 at 03:51 +0800, Sean Wang wrote:
> And the subject should be also corrected with prefix starting with
> "pinctrl: mediatek:", typo fixup, and having a better subject close to
> the content.
I will change it in next patch.
> On Thu, Dec 13, 2018 at 1:36 AM <chuanjia.liu@mediatek.com> wrote:
> >
> > From: Chuanjia Liu <Chuanjia.Liu@mediatek.com>
> >
> > This patch add gpio vritual number select,avoid virtual gpio set SMT.
> >
> > Signed-off-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com>
> > ---
> >  drivers/pinctrl/mediatek/mtk-eint.h              |    1 +
> >  drivers/pinctrl/mediatek/pinctrl-mt8183.c        |    1 +
> >  drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c |    9 ++++++---
> >  3 files changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pinctrl/mediatek/mtk-eint.h b/drivers/pinctrl/mediatek/mtk-eint.h
> > index 48468d0..c16beaf 100644
> > --- a/drivers/pinctrl/mediatek/mtk-eint.h
> > +++ b/drivers/pinctrl/mediatek/mtk-eint.h
> > @@ -37,6 +37,7 @@ struct mtk_eint_hw {
> >         u8              ports;
> >         unsigned int    ap_num;
> >         unsigned int    db_cnt;
> > +       unsigned int    vir_start;
> >  };
> >
> >  struct mtk_eint;
> > diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8183.c b/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> > index 6262fd3..bbeafd3 100644
> > --- a/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> > +++ b/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> > @@ -497,6 +497,7 @@
> >         .ports     = 6,
> >         .ap_num    = 212,
> >         .db_cnt    = 13,
> > +       .vir_start = 180,
> >  };
> >
> >  static const struct mtk_pin_soc mt8183_data = {
> > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> > index 4a9e0d4..ca3bae1 100644
> > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> > @@ -289,9 +289,12 @@ static int mtk_xt_set_gpio_as_eint(void *data, unsigned long eint_n)
> >         if (err)
> >                 return err;
> >
> > -       err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT, MTK_ENABLE);
> > -       if (err)
> > -               return err;
> > +       if (gpio_n < hw->eint->hw->vir_start) {
> > +               err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
> > +                                      MTK_ENABLE);
> > +               if (err)
> > +                       return err;
> > +       }
> >
> >         return 0;
> >  }
> > --
> > 1.7.9.5



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] eint: add gpio vritual number select
  2018-12-17  3:15   ` Chuanjia Liu
@ 2018-12-17  4:04     ` Yingjoe Chen
  2018-12-17 11:24       ` Chuanjia Liu
  2018-12-17  7:59     ` Sean Wang
  1 sibling, 1 reply; 8+ messages in thread
From: Yingjoe Chen @ 2018-12-17  4:04 UTC (permalink / raw)
  To: Chuanjia Liu
  Cc: Sean Wang, youlin.pei, hongkun.cao, zhiyong.tao, Linus Walleij,
	linux-kernel, linux-mediatek, hailong.fan, Matthias Brugger,
	eddie.huang, linux-arm-kernel

On Mon, 2018-12-17 at 11:15 +0800, Chuanjia Liu wrote:
> On Thu, 2018-12-13 at 11:33 -0800, Sean Wang wrote:
> > On Thu, Dec 13, 2018 at 1:36 AM <chuanjia.liu@mediatek.com> wrote:
> > >
> > > From: Chuanjia Liu <Chuanjia.Liu@mediatek.com>
> > >
> > > This patch add gpio vritual number select,avoid virtual gpio set SMT.
> > 
> > s/gpio/GPIO/
> > s/vritual/virtual/
> > 
> > Virtual GPIOs you said here that means these pins only used inside SoC
> > and not being exported to outside SoC, right? It seems this kind of
> > pins doesn't need SMT.
> > 
> Yes,virtual gpio only used inside SOC and these pins doesn't need set
> SMT

Hi,

I don't see full patch in linux-mediatek archive. Maybe you are not
subscribed so it is rejected?

Please add this description to commit message and/or code comment.
I think 'internal GPIO' might be a better name for this. Does the name
'virtual GPIO' come from datasheet?


> > >
> > > Signed-off-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com>
> > > ---
> > >  drivers/pinctrl/mediatek/mtk-eint.h              |    1 +
> > >  drivers/pinctrl/mediatek/pinctrl-mt8183.c        |    1 +
> > >  drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c |    9 ++++++---
> > >  3 files changed, 8 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/pinctrl/mediatek/mtk-eint.h b/drivers/pinctrl/mediatek/mtk-eint.h
> > > index 48468d0..c16beaf 100644
> > > --- a/drivers/pinctrl/mediatek/mtk-eint.h
> > > +++ b/drivers/pinctrl/mediatek/mtk-eint.h
> > > @@ -37,6 +37,7 @@ struct mtk_eint_hw {
> > >         u8              ports;
> > >         unsigned int    ap_num;
> > >         unsigned int    db_cnt;
> > > +       unsigned int    vir_start;


Since it is about GPIO and SMT, maybe it should be added to mtk_pin_soc
instead of mtk_eint_hw ?

Joe.C

> > >  };
> > >
> > >  struct mtk_eint;
> > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8183.c b/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> > > index 6262fd3..bbeafd3 100644
> > > --- a/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> > > +++ b/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> > > @@ -497,6 +497,7 @@
> > >         .ports     = 6,
> > >         .ap_num    = 212,
> > >         .db_cnt    = 13,
> > > +       .vir_start = 180,
> > >  };
> > >
> > >  static const struct mtk_pin_soc mt8183_data = {
> > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> > > index 4a9e0d4..ca3bae1 100644
> > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> > > @@ -289,9 +289,12 @@ static int mtk_xt_set_gpio_as_eint(void *data, unsigned long eint_n)
> > >         if (err)
> > >                 return err;
> > >
> > > -       err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT, MTK_ENABLE);
> > > -       if (err)
> > > -               return err;
> > > +       if (gpio_n < hw->eint->hw->vir_start) {
> > > +               err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
> > > +                                      MTK_ENABLE);
> > > +               if (err)
> > > +                       return err;
> > > +       }
> > 
> > The changes will break these SoCs without a properly configured vir_start.
> > 
> > If SMT seems unnecessary for these kinds of virtual GPIOs pin in the
> > path, we can do it as
> > 
> > err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
> >                                         MTK_ENABLE);
> > /* please add comments for the exclusion condition */
> > if (err && err != -ENOTSUPP)
> >         return err;
> > 
> > If there is getting much special on certain pins between SoCs, and
> > then we can consider creating a desc->flag to split logic.
> 
> Yes,SMT unnecessary for these kinds of virtual GPIOS pin in the path,if
> do it as
> 	err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
>                                         MTK_ENABLE);
> 	if (err && err != -ENOTSUPP)
>       		  return err;
> I wonder if system will lose -ENOTSUPP information when smt was not
> successfully set by real gpio?
> > 
> > >
> > >         return 0;
> > >  }
> > > --
> > > 1.7.9.5
> 
> 
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] eint: add gpio vritual number select
  2018-12-17  3:15   ` Chuanjia Liu
  2018-12-17  4:04     ` Yingjoe Chen
@ 2018-12-17  7:59     ` Sean Wang
  2018-12-17 11:09       ` Chuanjia Liu
  1 sibling, 1 reply; 8+ messages in thread
From: Sean Wang @ 2018-12-17  7:59 UTC (permalink / raw)
  To: Chuanjia.Liu
  Cc: Linus Walleij, Matthias Brugger, linux-mediatek,
	linux-arm-kernel, linux-kernel, hongkun.cao, youlin.pei,
	eddie.huang, zhiyong.tao, hailong.fan

On Sun, Dec 16, 2018 at 7:15 PM Chuanjia Liu <Chuanjia.Liu@mediatek.com> wrote:
>
> On Thu, 2018-12-13 at 11:33 -0800, Sean Wang wrote:
> > On Thu, Dec 13, 2018 at 1:36 AM <chuanjia.liu@mediatek.com> wrote:
> > >
> > > From: Chuanjia Liu <Chuanjia.Liu@mediatek.com>
> > >
> > > This patch add gpio vritual number select,avoid virtual gpio set SMT.
> >
> > s/gpio/GPIO/
> > s/vritual/virtual/
> >
> > Virtual GPIOs you said here that means these pins only used inside SoC
> > and not being exported to outside SoC, right? It seems this kind of
> > pins doesn't need SMT.
> >
> Yes,virtual gpio only used inside SOC and these pins doesn't need set
> SMT
> > >
> > > Signed-off-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com>
> > > ---
> > >  drivers/pinctrl/mediatek/mtk-eint.h              |    1 +
> > >  drivers/pinctrl/mediatek/pinctrl-mt8183.c        |    1 +
> > >  drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c |    9 ++++++---
> > >  3 files changed, 8 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/pinctrl/mediatek/mtk-eint.h b/drivers/pinctrl/mediatek/mtk-eint.h
> > > index 48468d0..c16beaf 100644
> > > --- a/drivers/pinctrl/mediatek/mtk-eint.h
> > > +++ b/drivers/pinctrl/mediatek/mtk-eint.h
> > > @@ -37,6 +37,7 @@ struct mtk_eint_hw {
> > >         u8              ports;
> > >         unsigned int    ap_num;
> > >         unsigned int    db_cnt;
> > > +       unsigned int    vir_start;
> > >  };
> > >
> > >  struct mtk_eint;
> > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8183.c b/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> > > index 6262fd3..bbeafd3 100644
> > > --- a/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> > > +++ b/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> > > @@ -497,6 +497,7 @@
> > >         .ports     = 6,
> > >         .ap_num    = 212,
> > >         .db_cnt    = 13,
> > > +       .vir_start = 180,
> > >  };
> > >
> > >  static const struct mtk_pin_soc mt8183_data = {
> > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> > > index 4a9e0d4..ca3bae1 100644
> > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> > > @@ -289,9 +289,12 @@ static int mtk_xt_set_gpio_as_eint(void *data, unsigned long eint_n)
> > >         if (err)
> > >                 return err;
> > >
> > > -       err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT, MTK_ENABLE);
> > > -       if (err)
> > > -               return err;
> > > +       if (gpio_n < hw->eint->hw->vir_start) {
> > > +               err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
> > > +                                      MTK_ENABLE);
> > > +               if (err)
> > > +                       return err;
> > > +       }
> >
> > The changes will break these SoCs without a properly configured vir_start.
> >
> > If SMT seems unnecessary for these kinds of virtual GPIOs pin in the
> > path, we can do it as
> >
> > err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
> >                                         MTK_ENABLE);
> > /* please add comments for the exclusion condition */
> > if (err && err != -ENOTSUPP)
> >         return err;
> >
> > If there is getting much special on certain pins between SoCs, and
> > then we can consider creating a desc->flag to split logic.
>
> Yes,SMT unnecessary for these kinds of virtual GPIOS pin in the path,if
> do it as
>         err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
>                                         MTK_ENABLE);
>         if (err && err != -ENOTSUPP)
>                   return err;
> I wonder if system will lose -ENOTSUPP information when smt was not
> successfully set by real gpio?

-ENOTSUPP shouldn't happen in a real pin as SMT is supposed to be
supported by every real pin.

If it is not true or there are more special on certain pins, and then
we consider to add a flag to struct mtk_pin_desc to group these pins
with their characteristics and to split and reuse the common code flow
with the extra flag.

So for now, I thought it's enough to handle your case with adding a
few well self-explained comments for the exclusion condition. These
words help us remember we should add adding an extra flag to pin
description as one of TODO if more needs like you come out.

> >
> > >
> > >         return 0;
> > >  }
> > > --
> > > 1.7.9.5
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] eint: add gpio vritual number select
  2018-12-17  7:59     ` Sean Wang
@ 2018-12-17 11:09       ` Chuanjia Liu
  0 siblings, 0 replies; 8+ messages in thread
From: Chuanjia Liu @ 2018-12-17 11:09 UTC (permalink / raw)
  To: Sean Wang
  Cc: Linus Walleij, Matthias Brugger, linux-mediatek,
	linux-arm-kernel, linux-kernel, hongkun.cao, youlin.pei,
	eddie.huang, zhiyong.tao, hailong.fan

On Sun, 2018-12-16 at 23:59 -0800, Sean Wang wrote:
> On Sun, Dec 16, 2018 at 7:15 PM Chuanjia Liu <Chuanjia.Liu@mediatek.com> wrote:
> >
> > On Thu, 2018-12-13 at 11:33 -0800, Sean Wang wrote:
> > > On Thu, Dec 13, 2018 at 1:36 AM <chuanjia.liu@mediatek.com> wrote:
> > > >
> > > > From: Chuanjia Liu <Chuanjia.Liu@mediatek.com>
> > > >
> > > > This patch add gpio vritual number select,avoid virtual gpio set SMT.
> > >
> > > s/gpio/GPIO/
> > > s/vritual/virtual/
> > >
> > > Virtual GPIOs you said here that means these pins only used inside SoC
> > > and not being exported to outside SoC, right? It seems this kind of
> > > pins doesn't need SMT.
> > >
> > Yes,virtual gpio only used inside SOC and these pins doesn't need set
> > SMT
> > > >
> > > > Signed-off-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com>
> > > > ---
> > > >  drivers/pinctrl/mediatek/mtk-eint.h              |    1 +
> > > >  drivers/pinctrl/mediatek/pinctrl-mt8183.c        |    1 +
> > > >  drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c |    9 ++++++---
> > > >  3 files changed, 8 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/pinctrl/mediatek/mtk-eint.h b/drivers/pinctrl/mediatek/mtk-eint.h
> > > > index 48468d0..c16beaf 100644
> > > > --- a/drivers/pinctrl/mediatek/mtk-eint.h
> > > > +++ b/drivers/pinctrl/mediatek/mtk-eint.h
> > > > @@ -37,6 +37,7 @@ struct mtk_eint_hw {
> > > >         u8              ports;
> > > >         unsigned int    ap_num;
> > > >         unsigned int    db_cnt;
> > > > +       unsigned int    vir_start;
> > > >  };
> > > >
> > > >  struct mtk_eint;
> > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8183.c b/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> > > > index 6262fd3..bbeafd3 100644
> > > > --- a/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> > > > +++ b/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> > > > @@ -497,6 +497,7 @@
> > > >         .ports     = 6,
> > > >         .ap_num    = 212,
> > > >         .db_cnt    = 13,
> > > > +       .vir_start = 180,
> > > >  };
> > > >
> > > >  static const struct mtk_pin_soc mt8183_data = {
> > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> > > > index 4a9e0d4..ca3bae1 100644
> > > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> > > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> > > > @@ -289,9 +289,12 @@ static int mtk_xt_set_gpio_as_eint(void *data, unsigned long eint_n)
> > > >         if (err)
> > > >                 return err;
> > > >
> > > > -       err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT, MTK_ENABLE);
> > > > -       if (err)
> > > > -               return err;
> > > > +       if (gpio_n < hw->eint->hw->vir_start) {
> > > > +               err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
> > > > +                                      MTK_ENABLE);
> > > > +               if (err)
> > > > +                       return err;
> > > > +       }
> > >
> > > The changes will break these SoCs without a properly configured vir_start.
> > >
> > > If SMT seems unnecessary for these kinds of virtual GPIOs pin in the
> > > path, we can do it as
> > >
> > > err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
> > >                                         MTK_ENABLE);
> > > /* please add comments for the exclusion condition */
> > > if (err && err != -ENOTSUPP)
> > >         return err;
> > >
> > > If there is getting much special on certain pins between SoCs, and
> > > then we can consider creating a desc->flag to split logic.
> >
> > Yes,SMT unnecessary for these kinds of virtual GPIOS pin in the path,if
> > do it as
> >         err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
> >                                         MTK_ENABLE);
> >         if (err && err != -ENOTSUPP)
> >                   return err;
> > I wonder if system will lose -ENOTSUPP information when smt was not
> > successfully set by real gpio?
> 
> -ENOTSUPP shouldn't happen in a real pin as SMT is supposed to be
> supported by every real pin.
> 
> If it is not true or there are more special on certain pins, and then
> we consider to add a flag to struct mtk_pin_desc to group these pins
> with their characteristics and to split and reuse the common code flow
> with the extra flag.
> 
> So for now, I thought it's enough to handle your case with adding a
> few well self-explained comments for the exclusion condition. These
> words help us remember we should add adding an extra flag to pin
> description as one of TODO if more needs like you come out.
> 
 Thanks for your advice,I will update new patch
> > >
> > > >
> > > >         return 0;
> > > >  }
> > > > --
> > > > 1.7.9.5
> >
> >



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] eint: add gpio vritual number select
  2018-12-17  4:04     ` Yingjoe Chen
@ 2018-12-17 11:24       ` Chuanjia Liu
  0 siblings, 0 replies; 8+ messages in thread
From: Chuanjia Liu @ 2018-12-17 11:24 UTC (permalink / raw)
  To: Yingjoe Chen
  Cc: Sean Wang, youlin.pei, hongkun.cao, zhiyong.tao, Linus Walleij,
	linux-kernel, linux-mediatek, hailong.fan, Matthias Brugger,
	eddie.huang, linux-arm-kernel

On Mon, 2018-12-17 at 12:04 +0800, Yingjoe Chen wrote:
> On Mon, 2018-12-17 at 11:15 +0800, Chuanjia Liu wrote:
> > On Thu, 2018-12-13 at 11:33 -0800, Sean Wang wrote:
> > > On Thu, Dec 13, 2018 at 1:36 AM <chuanjia.liu@mediatek.com> wrote:
> > > >
> > > > From: Chuanjia Liu <Chuanjia.Liu@mediatek.com>
> > > >
> > > > This patch add gpio vritual number select,avoid virtual gpio set SMT.
> > > 
> > > s/gpio/GPIO/
> > > s/vritual/virtual/
> > > 
> > > Virtual GPIOs you said here that means these pins only used inside SoC
> > > and not being exported to outside SoC, right? It seems this kind of
> > > pins doesn't need SMT.
> > > 
> > Yes,virtual gpio only used inside SOC and these pins doesn't need set
> > SMT
> 
> Hi,
> 
> I don't see full patch in linux-mediatek archive. Maybe you are not
> subscribed so it is rejected?
Some groups failed to send because of permission issues,I will update
new patch.
> 
> Please add this description to commit message and/or code comment.
> I think 'internal GPIO' might be a better name for this. Does the name
> 'virtual GPIO' come from datasheet?
MTKer call it virtyal gpio When some pins only used inside soc.
> 
> 
> > > >
> > > > Signed-off-by: Chuanjia Liu <Chuanjia.Liu@mediatek.com>
> > > > ---
> > > >  drivers/pinctrl/mediatek/mtk-eint.h              |    1 +
> > > >  drivers/pinctrl/mediatek/pinctrl-mt8183.c        |    1 +
> > > >  drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c |    9 ++++++---
> > > >  3 files changed, 8 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/pinctrl/mediatek/mtk-eint.h b/drivers/pinctrl/mediatek/mtk-eint.h
> > > > index 48468d0..c16beaf 100644
> > > > --- a/drivers/pinctrl/mediatek/mtk-eint.h
> > > > +++ b/drivers/pinctrl/mediatek/mtk-eint.h
> > > > @@ -37,6 +37,7 @@ struct mtk_eint_hw {
> > > >         u8              ports;
> > > >         unsigned int    ap_num;
> > > >         unsigned int    db_cnt;
> > > > +       unsigned int    vir_start;

> Since it is about GPIO and SMT, maybe it should be added to mtk_pin_soc
> instead of mtk_eint_hw ?
> 
> Joe.C

I will delete this change,thanks
> > > >  };
> > > >
> > > >  struct mtk_eint;
> > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8183.c b/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> > > > index 6262fd3..bbeafd3 100644
> > > > --- a/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> > > > +++ b/drivers/pinctrl/mediatek/pinctrl-mt8183.c
> > > > @@ -497,6 +497,7 @@
> > > >         .ports     = 6,
> > > >         .ap_num    = 212,
> > > >         .db_cnt    = 13,
> > > > +       .vir_start = 180,
> > > >  };
> > > >
> > > >  static const struct mtk_pin_soc mt8183_data = {
> > > > diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> > > > index 4a9e0d4..ca3bae1 100644
> > > > --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> > > > +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c
> > > > @@ -289,9 +289,12 @@ static int mtk_xt_set_gpio_as_eint(void *data, unsigned long eint_n)
> > > >         if (err)
> > > >                 return err;
> > > >
> > > > -       err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT, MTK_ENABLE);
> > > > -       if (err)
> > > > -               return err;
> > > > +       if (gpio_n < hw->eint->hw->vir_start) {
> > > > +               err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
> > > > +                                      MTK_ENABLE);
> > > > +               if (err)
> > > > +                       return err;
> > > > +       }
> > > 
> > > The changes will break these SoCs without a properly configured vir_start.
> > > 
> > > If SMT seems unnecessary for these kinds of virtual GPIOs pin in the
> > > path, we can do it as
> > > 
> > > err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
> > >                                         MTK_ENABLE);
> > > /* please add comments for the exclusion condition */
> > > if (err && err != -ENOTSUPP)
> > >         return err;
> > > 
> > > If there is getting much special on certain pins between SoCs, and
> > > then we can consider creating a desc->flag to split logic.
> > 
> > Yes,SMT unnecessary for these kinds of virtual GPIOS pin in the path,if
> > do it as
> > 	err = mtk_hw_set_value(hw, desc, PINCTRL_PIN_REG_SMT,
> >                                         MTK_ENABLE);
> > 	if (err && err != -ENOTSUPP)
> >       		  return err;
> > I wonder if system will lose -ENOTSUPP information when smt was not
> > successfully set by real gpio?
> > > 
> > > >
> > > >         return 0;
> > > >  }
> > > > --
> > > > 1.7.9.5
> > 
> > 
> > 
> > _______________________________________________
> > Linux-mediatek mailing list
> > Linux-mediatek@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-mediatek
> 
> 



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-12-17 11:24 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1544693783-25079-1-git-send-email-chuanjia.liu@mediatek.com>
2018-12-13 19:33 ` [PATCH] eint: add gpio vritual number select Sean Wang
2018-12-17  3:15   ` Chuanjia Liu
2018-12-17  4:04     ` Yingjoe Chen
2018-12-17 11:24       ` Chuanjia Liu
2018-12-17  7:59     ` Sean Wang
2018-12-17 11:09       ` Chuanjia Liu
2018-12-13 19:51 ` Sean Wang
2018-12-17  3:19   ` Chuanjia Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).