All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yingjoe Chen <yingjoe.chen@mediatek.com>
To: Chuanjia Liu <Chuanjia.Liu@mediatek.com>
Cc: youlin.pei@mediatek.com, hongkun.cao@mediatek.com,
	zhiyong.tao@mediatek.com,
	Linus Walleij <linus.walleij@linaro.org>,
	Sean Wang <sean.wang@kernel.org>,
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	hailong.fan@mediatek.com,
	Matthias Brugger <matthias.bgg@gmail.com>,
	eddie.huang@mediatek.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] eint: add gpio vritual number select
Date: Mon, 17 Dec 2018 12:04:42 +0800	[thread overview]
Message-ID: <1545019482.27863.8.camel@mtksdaap41> (raw)
In-Reply-To: <1545016504.29293.10.camel@mhfsdcap03>

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

WARNING: multiple messages have this Message-ID (diff)
From: Yingjoe Chen <yingjoe.chen@mediatek.com>
To: Chuanjia Liu <Chuanjia.Liu@mediatek.com>
Cc: youlin.pei@mediatek.com, hongkun.cao@mediatek.com,
	zhiyong.tao@mediatek.com,
	Linus Walleij <linus.walleij@linaro.org>,
	Sean Wang <sean.wang@kernel.org>,
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	hailong.fan@mediatek.com,
	Matthias Brugger <matthias.bgg@gmail.com>,
	eddie.huang@mediatek.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] eint: add gpio vritual number select
Date: Mon, 17 Dec 2018 12:04:42 +0800	[thread overview]
Message-ID: <1545019482.27863.8.camel@mtksdaap41> (raw)
In-Reply-To: <1545016504.29293.10.camel@mhfsdcap03>

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



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Yingjoe Chen <yingjoe.chen@mediatek.com>
To: Chuanjia Liu <Chuanjia.Liu@mediatek.com>
Cc: Sean Wang <sean.wang@kernel.org>, <youlin.pei@mediatek.com>,
	<hongkun.cao@mediatek.com>, <zhiyong.tao@mediatek.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	<linux-kernel@vger.kernel.org>,
	<linux-mediatek@lists.infradead.org>, <hailong.fan@mediatek.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	<eddie.huang@mediatek.com>,
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] eint: add gpio vritual number select
Date: Mon, 17 Dec 2018 12:04:42 +0800	[thread overview]
Message-ID: <1545019482.27863.8.camel@mtksdaap41> (raw)
In-Reply-To: <1545016504.29293.10.camel@mhfsdcap03>

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



  reply	other threads:[~2018-12-17  4:04 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [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:33   ` Sean Wang
2018-12-17  3:15   ` Chuanjia Liu
2018-12-17  3:15     ` Chuanjia Liu
2018-12-17  3:15     ` Chuanjia Liu
2018-12-17  4:04     ` Yingjoe Chen [this message]
2018-12-17  4:04       ` Yingjoe Chen
2018-12-17  4:04       ` Yingjoe Chen
2018-12-17 11:24       ` Chuanjia Liu
2018-12-17 11:24         ` Chuanjia Liu
2018-12-17 11:24         ` Chuanjia Liu
2018-12-17  7:59     ` Sean Wang
2018-12-17  7:59       ` Sean Wang
2018-12-17 11:09       ` Chuanjia Liu
2018-12-17 11:09         ` Chuanjia Liu
2018-12-17 11:09         ` Chuanjia Liu
2018-12-13 19:51 ` Sean Wang
2018-12-13 19:51   ` Sean Wang
2018-12-17  3:19   ` Chuanjia Liu
2018-12-17  3:19     ` Chuanjia Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1545019482.27863.8.camel@mtksdaap41 \
    --to=yingjoe.chen@mediatek.com \
    --cc=Chuanjia.Liu@mediatek.com \
    --cc=eddie.huang@mediatek.com \
    --cc=hailong.fan@mediatek.com \
    --cc=hongkun.cao@mediatek.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=sean.wang@kernel.org \
    --cc=youlin.pei@mediatek.com \
    --cc=zhiyong.tao@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.