Re: [PATCH] eint: add gpio vritual number select

2018-12-17 Thread Chuanjia Liu
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  wrote:
> > > >
> > > > From: Chuanjia Liu 
> > > >
> > > > 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 
> > > > ---
> > > >  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 intap_num;
> > > > unsigned intdb_cnt;
> > > > +   unsigned intvir_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-media...@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-mediatek
> 
> 




Re: [PATCH] eint: add gpio vritual number select

2018-12-17 Thread Chuanjia Liu
On Sun, 2018-12-16 at 23:59 -0800, Sean Wang wrote:
> On Sun, Dec 16, 2018 at 7:15 PM Chuanjia Liu  
> wrote:
> >
> > On Thu, 2018-12-13 at 11:33 -0800, Sean Wang wrote:
> > > On Thu, Dec 13, 2018 at 1:36 AM  wrote:
> > > >
> > > > From: Chuanjia Liu 
> > > >
> > > > 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 
> > > > ---
> > > >  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 intap_num;
> > > > unsigned intdb_cnt;
> > > > +   unsigned intvir_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
> >
> >




Re: [PATCH] eint: add gpio vritual number select

2018-12-17 Thread Sean Wang
On Sun, Dec 16, 2018 at 7:15 PM Chuanjia Liu  wrote:
>
> On Thu, 2018-12-13 at 11:33 -0800, Sean Wang wrote:
> > On Thu, Dec 13, 2018 at 1:36 AM  wrote:
> > >
> > > From: Chuanjia Liu 
> > >
> > > 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 
> > > ---
> > >  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 intap_num;
> > > unsigned intdb_cnt;
> > > +   unsigned intvir_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
>
>


Re: [PATCH] eint: add gpio vritual number select

2018-12-16 Thread Yingjoe Chen
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  wrote:
> > >
> > > From: Chuanjia Liu 
> > >
> > > 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 
> > > ---
> > >  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 intap_num;
> > > unsigned intdb_cnt;
> > > +   unsigned intvir_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-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek




Re: [PATCH] eint: add gpio vritual number select

2018-12-16 Thread Chuanjia Liu
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  wrote:
> >
> > From: Chuanjia Liu 
> >
> > This patch add gpio vritual number select,avoid virtual gpio set SMT.
> >
> > Signed-off-by: Chuanjia Liu 
> > ---
> >  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 intap_num;
> > unsigned intdb_cnt;
> > +   unsigned intvir_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




Re: [PATCH] eint: add gpio vritual number select

2018-12-16 Thread Chuanjia Liu
On Thu, 2018-12-13 at 11:33 -0800, Sean Wang wrote:
> On Thu, Dec 13, 2018 at 1:36 AM  wrote:
> >
> > From: Chuanjia Liu 
> >
> > 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 
> > ---
> >  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 intap_num;
> > unsigned intdb_cnt;
> > +   unsigned intvir_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




Re: [PATCH] eint: add gpio vritual number select

2018-12-13 Thread Sean Wang
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  wrote:
>
> From: Chuanjia Liu 
>
> This patch add gpio vritual number select,avoid virtual gpio set SMT.
>
> Signed-off-by: Chuanjia Liu 
> ---
>  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 intap_num;
> unsigned intdb_cnt;
> +   unsigned intvir_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


Re: [PATCH] eint: add gpio vritual number select

2018-12-13 Thread Sean Wang
On Thu, Dec 13, 2018 at 1:36 AM  wrote:
>
> From: Chuanjia Liu 
>
> 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 
> ---
>  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 intap_num;
> unsigned intdb_cnt;
> +   unsigned intvir_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