mbox series

[V3,0/4] gpio: eic-sprd: Modification of UNISOC Platform EIC Driver

Message ID 20240104024244.12163-1-Wenhua.Lin@unisoc.com
Headers show
Series gpio: eic-sprd: Modification of UNISOC Platform EIC Driver | expand

Message

Wenhua Lin Jan. 4, 2024, 2:42 a.m. UTC 
Recently, some bugs have been discovered during use, and
patch2 are bug fixes. Also, this patchset add optimization:
patch1 can support eic debouce wake-up system and
patch3 optimization the calculation method of eic number,
and patch4 Support 8 banks EIC controller.

Change in V3:
-Using thread send 4 patches 

-Change title and commit message in PATCH 1/4.
-Delete fixes tag in PATCH 1/4.

-Change commit message in PATCH 2/4.

-Move num_banks++ to the back of sprd_eic->base in PATCH 3/4.
-Delete fixes tag in PATCH 3/4.
-Modify misindented issue in PATCH 3/4.
-Preserve reversed xmas tree order in PATCH 3/4.

-Change related comments in PATCH 4/4.

Wenhua Lin (4):
  gpio: eic-sprd: Keep the clock rtc_1k on
  gpio: eic-sprd: Clear interrupt after set the interrupt type
  gpio: eic-sprd: Modify the calculation method of eic number
  gpio: eic-sprd: Support 8 banks EIC controller

 drivers/gpio/gpio-eic-sprd.c | 65 +++++++++++++++++++++---------------
 1 file changed, 39 insertions(+), 26 deletions(-)

Comments

Andy Shevchenko Jan. 4, 2024, 12:59 p.m. UTC | #1 
On Thu, Jan 4, 2024 at 4:43 AM Wenhua Lin <Wenhua.Lin@unisoc.com> wrote:
>
> The eic debounce does not have a clock of rtc_1k in the sleep state,
> but the eic debounce will be used to wake up the system, therefore the
> clock of rtc_1k needs to be kept open.

...

> +#define SPRD_EIC_DBNC_FORCE_CLK                0x8000

BIT(15) ?
wenhua lin Jan. 5, 2024, 2:11 a.m. UTC | #2 
On Thu, Jan 4, 2024 at 9:00 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Thu, Jan 4, 2024 at 4:43 AM Wenhua Lin <Wenhua.Lin@unisoc.com> wrote:
> >
> > The eic debounce does not have a clock of rtc_1k in the sleep state,
> > but the eic debounce will be used to wake up the system, therefore the
> > clock of rtc_1k needs to be kept open.
>
> ...
>
> > +#define SPRD_EIC_DBNC_FORCE_CLK                0x8000
>
> BIT(15) ?
>

Yes, writing 1 to bit15 of the register can ensure that the clock of
rtc_1k remains normally on.

> --
> With Best Regards,
> Andy Shevchenko
wenhua lin Jan. 5, 2024, 2:13 a.m. UTC | #3 
On Thu, Jan 4, 2024 at 9:01 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Thu, Jan 4, 2024 at 4:43 AM Wenhua Lin <Wenhua.Lin@unisoc.com> wrote:
> >
> > When the soc changes, the corresponding gpio-eic-sprd.c
>
> SoC
>

Thank you very much for your review.
I will fix this issue in patch v4.

> > code needs to be modified, and the corresponding
> > Document must also be modified, which is quite troublesome.
> > To avoid modifying the driver file, the number of eics
> > is automatically calculated by matching dts nodes.
>
> --
> With Best Regards,
> Andy Shevchenko
Chunyan Zhang Jan. 5, 2024, 2:17 a.m. UTC | #4 
On Fri, 5 Jan 2024 at 10:11, wenhua lin <wenhua.lin1994@gmail.com> wrote:
>
> On Thu, Jan 4, 2024 at 9:00 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Thu, Jan 4, 2024 at 4:43 AM Wenhua Lin <Wenhua.Lin@unisoc.com> wrote:
> > >
> > > The eic debounce does not have a clock of rtc_1k in the sleep state,
> > > but the eic debounce will be used to wake up the system, therefore the
> > > clock of rtc_1k needs to be kept open.
> >
> > ...
> >
> > > +#define SPRD_EIC_DBNC_FORCE_CLK                0x8000
> >
> > BIT(15) ?
> >
>
> Yes, writing 1 to bit15 of the register can ensure that the clock of
> rtc_1k remains normally on.

Andy's comment means that you should use BIT(15) instead of 0x8000.

>
> > --
> > With Best Regards,
> > Andy Shevchenko
wenhua lin Jan. 5, 2024, 2:28 a.m. UTC | #5 
On Fri, Jan 5, 2024 at 10:18 AM Chunyan Zhang <zhang.lyra@gmail.com> wrote:
>
> On Fri, 5 Jan 2024 at 10:11, wenhua lin <wenhua.lin1994@gmail.com> wrote:
> >
> > On Thu, Jan 4, 2024 at 9:00 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > >
> > > On Thu, Jan 4, 2024 at 4:43 AM Wenhua Lin <Wenhua.Lin@unisoc.com> wrote:
> > > >
> > > > The eic debounce does not have a clock of rtc_1k in the sleep state,
> > > > but the eic debounce will be used to wake up the system, therefore the
> > > > clock of rtc_1k needs to be kept open.
> > >
> > > ...
> > >
> > > > +#define SPRD_EIC_DBNC_FORCE_CLK                0x8000
> > >
> > > BIT(15) ?
> > >
> >
> > Yes, writing 1 to bit15 of the register can ensure that the clock of
> > rtc_1k remains normally on.
>
> Andy's comment means that you should use BIT(15) instead of 0x8000.
>

OK, thank you very much for your explanation, I will make changes in patch v4.

> >
> > > --
> > > With Best Regards,
> > > Andy Shevchenko
Chunyan Zhang Jan. 5, 2024, 3:25 a.m. UTC | #6 
On Thu, 4 Jan 2024 at 10:43, Wenhua Lin <Wenhua.Lin@unisoc.com> wrote:
>
> The eic debounce does not have a clock of rtc_1k in the sleep state,
> but the eic debounce will be used to wake up the system, therefore the
> clock of rtc_1k needs to be kept open.

It seems that this issue is not in the latest SoCs. I would suggest
not changing for the time being.

Thanks,
Chunyan


>
> Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
> ---
>  drivers/gpio/gpio-eic-sprd.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpio/gpio-eic-sprd.c b/drivers/gpio/gpio-eic-sprd.c
> index be7f2fa5aa7b..bdcb3510a208 100644
> --- a/drivers/gpio/gpio-eic-sprd.c
> +++ b/drivers/gpio/gpio-eic-sprd.c
> @@ -24,6 +24,7 @@
>  #define SPRD_EIC_DBNC_IC               0x24
>  #define SPRD_EIC_DBNC_TRIG             0x28
>  #define SPRD_EIC_DBNC_CTRL0            0x40
> +#define SPRD_EIC_DBNC_FORCE_CLK                0x8000
>
>  #define SPRD_EIC_LATCH_INTEN           0x0
>  #define SPRD_EIC_LATCH_INTRAW          0x4
> @@ -223,6 +224,7 @@ static int sprd_eic_set_debounce(struct gpio_chip *chip, unsigned int offset,
>         u32 value = readl_relaxed(base + reg) & ~SPRD_EIC_DBNC_MASK;
>
>         value |= (debounce / 1000) & SPRD_EIC_DBNC_MASK;
> +       value |= SPRD_EIC_DBNC_FORCE_CLK;
>         writel_relaxed(value, base + reg);
>
>         return 0;
> --
> 2.17.1
>
wenhua lin Jan. 8, 2024, 7:36 a.m. UTC | #7 
On Fri, Jan 5, 2024 at 11:26 AM Chunyan Zhang <zhang.lyra@gmail.com> wrote:
>
> On Thu, 4 Jan 2024 at 10:43, Wenhua Lin <Wenhua.Lin@unisoc.com> wrote:
> >
> > The eic debounce does not have a clock of rtc_1k in the sleep state,
> > but the eic debounce will be used to wake up the system, therefore the
> > clock of rtc_1k needs to be kept open.
>
> It seems that this issue is not in the latest SoCs. I would suggest
> not changing for the time being.
>

OK, we will delete this modification on pacth v4.

> Thanks,
> Chunyan
>
>
> >
> > Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
> > ---
> >  drivers/gpio/gpio-eic-sprd.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpio/gpio-eic-sprd.c b/drivers/gpio/gpio-eic-sprd.c
> > index be7f2fa5aa7b..bdcb3510a208 100644
> > --- a/drivers/gpio/gpio-eic-sprd.c
> > +++ b/drivers/gpio/gpio-eic-sprd.c
> > @@ -24,6 +24,7 @@
> >  #define SPRD_EIC_DBNC_IC               0x24
> >  #define SPRD_EIC_DBNC_TRIG             0x28
> >  #define SPRD_EIC_DBNC_CTRL0            0x40
> > +#define SPRD_EIC_DBNC_FORCE_CLK                0x8000
> >
> >  #define SPRD_EIC_LATCH_INTEN           0x0
> >  #define SPRD_EIC_LATCH_INTRAW          0x4
> > @@ -223,6 +224,7 @@ static int sprd_eic_set_debounce(struct gpio_chip *chip, unsigned int offset,
> >         u32 value = readl_relaxed(base + reg) & ~SPRD_EIC_DBNC_MASK;
> >
> >         value |= (debounce / 1000) & SPRD_EIC_DBNC_MASK;
> > +       value |= SPRD_EIC_DBNC_FORCE_CLK;
> >         writel_relaxed(value, base + reg);
> >
> >         return 0;
> > --
> > 2.17.1
> >