Re: [PATCH v5 13/20] pinctrl: samsung: Add filter selection support for alive banks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Will,

Thanks for testing!

On Fri, 1 Dec 2023 at 23:55, William McVicker <willmcvicker@xxxxxxxxxx> wrote:
>
> On 12/01/2023, Peter Griffin wrote:
> > Newer Exynos SoCs have a filter selection register on alive bank pins.
> > This allows the selection of a digital or delay filter for each pin. If
> > the filter selection register is not available then the default filter
> > (digital) is applied.
> >
> > On suspend we apply the analog filter to all pins in the bank (as the
> > digital filter relies on a clock). On resume the digital filter is
> > reapplied to all pins in the bank. The digital filter is working via
> > clock and has an adjustable filter delay register bitfield, whereas
> > the analog filter uses a fixed delay.
> >
> > The filter determines to what extent signal fluctuations received through
> > the pad are considered glitches.
> >
> > The code path can be exercised using
> > echo mem > /sys/power/state
> > And then wake the device using a eint gpio
> >
> > Signed-off-by: Peter Griffin <peter.griffin@xxxxxxxxxx>
>
> Tested-by: Will McVicker <willmcvicker@xxxxxxxxxx>
>
> --
>
> I verified boot and that the pinctrl driver is probing. When I tested
>
>   echo mem > /sys/power/state
>
> I wasn't able to re-wake the device (not sure how to send an eint gpio on
> a phone form factor). I tried with no_console_suspend but that hits a SError
> Interrupt (likely due to a PMU secure register access). Let me know if there's
> anything else I should test out.

Yes you're correct that SError is caused by the PMU secure register access.

Thanks,

Peter.
>
> Thanks,
> Will
>
> > ---
> >  drivers/pinctrl/samsung/pinctrl-exynos.c  | 89 ++++++++++++++++++++++-
> >  drivers/pinctrl/samsung/pinctrl-exynos.h  |  7 ++
> >  drivers/pinctrl/samsung/pinctrl-samsung.c |  2 +
> >  drivers/pinctrl/samsung/pinctrl-samsung.h | 22 ++++++
> >  4 files changed, 119 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c
> > index 6b58ec84e34b..56fc11a1fe2f 100644
> > --- a/drivers/pinctrl/samsung/pinctrl-exynos.c
> > +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c
> > @@ -269,6 +269,71 @@ struct exynos_eint_gpio_save {
> >       u32 eint_mask;
> >  };
> >
> > +/*
> > + * Set the desired filter (digital or analog delay) to every pin in
> > + * the bank. Note the filter selection bitfield is only found on alive
> > + * banks. The filter determines to what extent signal fluctuations
> > + * received through the pad are considered glitches.
> > + */
> > +static void exynos_eint_flt_config(struct samsung_pinctrl_drv_data *d,
> > +                                struct samsung_pin_bank *bank, int filter)
> > +{
> > +     unsigned int flt_reg, flt_con = 0;
> > +     unsigned int val, shift;
> > +     int i;
> > +     int loop_cnt;
> > +
> > +     /*
> > +      * The FLTCON register has the following layout
> > +      *
> > +      * BitfieldName[PinNum][Bit:Bit]
> > +      * FLT_EN[3][31] FLT_SEL[3][30] FLT_WIDTH[3][29:24]
> > +      * FLT_EN[2][23] FLT_SEL[2][22] FLT_WIDTH[2][21:16]
> > +      * FLT_EN[1][15] FLT_SEL[1][14] FLT_WIDTH[1][13:8]
> > +      * FLT_EN[0][7]  FLT_SEL[0][6]  FLT_WIDTH[0][5:0]
> > +      *
> > +      * FLT_EN       0x0 = Disable, 0x1=Enable
> > +      * FLT_SEL      0x0 = Delay filter, 0x1 Digital filter
> > +      * FLT_WIDTH    Filtering width. Valid when FLT_SEL is 0x1
> > +      */
> > +
> > +     flt_con |= EXYNOS9_FLTCON_EN;
> > +
> > +     if (filter)
> > +             flt_con |= EXYNOS9_FLTCON_DIGITAL;
> > +
> > +     flt_reg = EXYNOS_GPIO_EFLTCON_OFFSET + bank->fltcon_offset;
> > +
> > +     /*
> > +      * If nr_pins > 4, we should set FLTCON0 register fully.
> > +      * (pin0 ~ 3). So loop 4 times in case of FLTCON0.
> > +      */
> > +     if (bank->nr_pins > EXYNOS9_FLTCON_NR_PIN)
> > +             loop_cnt = EXYNOS9_FLTCON_NR_PIN;
> > +     else
> > +             loop_cnt = bank->nr_pins;
> > +
> > +     val = readl(d->virt_base + flt_reg);
> > +     for (i = 0; i < loop_cnt; i++) {
> > +             shift = i * EXYNOS9_FLTCON_LEN;
> > +             val &= ~(EXYNOS9_FLTCON_MASK << shift);
> > +             val |= (flt_con << shift);
> > +     }
> > +     writel(val, d->virt_base + flt_reg);
> > +
> > +     /* Loop for FLTCON1 pin 4 ~ 7 */
> > +     if (bank->nr_pins > EXYNOS9_FLTCON_NR_PIN) {
> > +             loop_cnt = (bank->nr_pins - EXYNOS9_FLTCON_NR_PIN);
> > +             val = readl(d->virt_base + flt_reg + 0x4);
> > +             for (i = 0; i < loop_cnt; i++) {
> > +                     shift = i * EXYNOS9_FLTCON_LEN;
> > +                     val &= ~(EXYNOS9_FLTCON_MASK << shift);
> > +                     val |= (flt_con << shift);
> > +             }
> > +             writel(val, d->virt_base + flt_reg + 0x4);
> > +     }
> > +}
> > +
> >  /*
> >   * exynos_eint_gpio_init() - setup handling of external gpio interrupts.
> >   * @d: driver data of samsung pinctrl driver.
> > @@ -321,6 +386,10 @@ __init int exynos_eint_gpio_init(struct samsung_pinctrl_drv_data *d)
> >                       goto err_domains;
> >               }
> >
> > +             /* Set Delay Analog Filter */
> > +             if (bank->fltcon_type != FLT_DEFAULT)
> > +                     exynos_eint_flt_config(d, bank,
> > +                                            EXYNOS9_FLTCON_DELAY);
> >       }
> >
> >       return 0;
> > @@ -555,6 +624,11 @@ __init int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d)
> >               if (bank->eint_type != EINT_TYPE_WKUP)
> >                       continue;
> >
> > +             /* Set Digital Filter */
> > +             if (bank->fltcon_type != FLT_DEFAULT)
> > +                     exynos_eint_flt_config(d, bank,
> > +                                            EXYNOS9_FLTCON_DIGITAL);
> > +
> >               bank->irq_chip = devm_kmemdup(dev, irq_chip, sizeof(*irq_chip),
> >                                             GFP_KERNEL);
> >               if (!bank->irq_chip) {
> > @@ -658,6 +732,7 @@ static void exynos_pinctrl_suspend_bank(
> >  void exynos_pinctrl_suspend(struct samsung_pinctrl_drv_data *drvdata)
> >  {
> >       struct samsung_pin_bank *bank = drvdata->pin_banks;
> > +     struct samsung_pinctrl_drv_data *d = bank->drvdata;
> >       struct exynos_irq_chip *irq_chip = NULL;
> >       int i;
> >
> > @@ -665,6 +740,10 @@ void exynos_pinctrl_suspend(struct samsung_pinctrl_drv_data *drvdata)
> >               if (bank->eint_type == EINT_TYPE_GPIO)
> >                       exynos_pinctrl_suspend_bank(drvdata, bank);
> >               else if (bank->eint_type == EINT_TYPE_WKUP) {
> > +                     /* Setting Delay (Analog) Filter */
> > +                     if (bank->fltcon_type != FLT_DEFAULT)
> > +                             exynos_eint_flt_config(d, bank,
> > +                                                    EXYNOS9_FLTCON_DELAY);
> >                       if (!irq_chip) {
> >                               irq_chip = bank->irq_chip;
> >                               irq_chip->set_eint_wakeup_mask(drvdata,
> > @@ -707,11 +786,19 @@ static void exynos_pinctrl_resume_bank(
> >  void exynos_pinctrl_resume(struct samsung_pinctrl_drv_data *drvdata)
> >  {
> >       struct samsung_pin_bank *bank = drvdata->pin_banks;
> > +     struct samsung_pinctrl_drv_data *d = bank->drvdata;
> >       int i;
> >
> >       for (i = 0; i < drvdata->nr_banks; ++i, ++bank)
> > -             if (bank->eint_type == EINT_TYPE_GPIO)
> > +             if (bank->eint_type == EINT_TYPE_GPIO) {
> >                       exynos_pinctrl_resume_bank(drvdata, bank);
> > +             } else if (bank->eint_type == EINT_TYPE_WKUP ||
> > +                        bank->eint_type == EINT_TYPE_WKUP_MUX) {
> > +                     /* Set Digital Filter */
> > +                     if (bank->fltcon_type != FLT_DEFAULT)
> > +                             exynos_eint_flt_config(d, bank,
> > +                                                    EXYNOS9_FLTCON_DIGITAL);
> > +             }
> >  }
> >
> >  static void exynos_retention_enable(struct samsung_pinctrl_drv_data *drvdata)
> > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h b/drivers/pinctrl/samsung/pinctrl-exynos.h
> > index 3ac52c2cf998..e2799ff1b5e9 100644
> > --- a/drivers/pinctrl/samsung/pinctrl-exynos.h
> > +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h
> > @@ -50,6 +50,13 @@
> >
> >  #define EXYNOS_EINT_MAX_PER_BANK     8
> >  #define EXYNOS_EINT_NR_WKUP_EINT
> > +/* EINT filter configuration */
> > +#define EXYNOS9_FLTCON_EN            BIT(7)
> > +#define EXYNOS9_FLTCON_DIGITAL               BIT(6)
> > +#define EXYNOS9_FLTCON_DELAY         (0 << 6)
> > +#define EXYNOS9_FLTCON_MASK          0xff
> > +#define EXYNOS9_FLTCON_LEN           8
> > +#define EXYNOS9_FLTCON_NR_PIN                4
> >
> >  #define EXYNOS_PIN_BANK_EINTN(pins, reg, id)         \
> >       {                                               \
> > diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c
> > index 79babbb39ced..50c360b4753a 100644
> > --- a/drivers/pinctrl/samsung/pinctrl-samsung.c
> > +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
> > @@ -1105,6 +1105,8 @@ samsung_pinctrl_get_soc_data(struct samsung_pinctrl_drv_data *d,
> >               bank->eint_func = bdata->eint_func;
> >               bank->eint_type = bdata->eint_type;
> >               bank->eint_mask = bdata->eint_mask;
> > +             bank->fltcon_type = bdata->fltcon_type;
> > +             bank->fltcon_offset = bdata->fltcon_offset;
> >               bank->eint_offset = bdata->eint_offset;
> >               bank->name = bdata->name;
> >
> > diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h b/drivers/pinctrl/samsung/pinctrl-samsung.h
> > index 9b3db50adef3..5fab3885a7d7 100644
> > --- a/drivers/pinctrl/samsung/pinctrl-samsung.h
> > +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h
> > @@ -82,6 +82,20 @@ enum eint_type {
> >       EINT_TYPE_WKUP_MUX,
> >  };
> >
> > +/**
> > + * enum fltcon_type - filter selection
> > + * @FLT_DEFAULT: filter not selectable, default digital filter
> > + * @FLT_SELECT: filter selectable (digital or delay)
> > + *
> > + * Some banks on newer Exynos based SoCs have a selectable filter on alive
> > + * banks of 'analog/delay' or 'digital'. If the filter selection register is
> > + * not available then the default filter is used (digital).
> > + */
> > +enum fltcon_type {
> > +     FLT_DEFAULT,
> > +     FLT_SELECTABLE,
> > +};
> > +
> >  /* maximum length of a pin in pin descriptor (example: "gpa0-0") */
> >  #define PIN_NAME_LENGTH      10
> >
> > @@ -122,6 +136,8 @@ struct samsung_pin_bank_type {
> >   * @eint_type: type of the external interrupt supported by the bank.
> >   * @eint_mask: bit mask of pins which support EINT function.
> >   * @eint_offset: SoC-specific EINT register or interrupt offset of bank.
> > + * @fltcon_type: whether the filter (delay/digital) is selectable
> > + * @fltcon_offset: SoC-specific EINT filter control register offset of bank.
> >   * @name: name to be prefixed for each pin in this pin bank.
> >   */
> >  struct samsung_pin_bank_data {
> > @@ -133,6 +149,8 @@ struct samsung_pin_bank_data {
> >       enum eint_type  eint_type;
> >       u32             eint_mask;
> >       u32             eint_offset;
> > +     enum fltcon_type fltcon_type;
> > +     u32             fltcon_offset;
> >       const char      *name;
> >  };
> >
> > @@ -147,6 +165,8 @@ struct samsung_pin_bank_data {
> >   * @eint_type: type of the external interrupt supported by the bank.
> >   * @eint_mask: bit mask of pins which support EINT function.
> >   * @eint_offset: SoC-specific EINT register or interrupt offset of bank.
> > + * @fltcon_type: whether the filter (delay/digital) is selectable
> > + * @fltcon_offset: SoC-specific EINT filter control register offset of bank.
> >   * @name: name to be prefixed for each pin in this pin bank.
> >   * @id: id of the bank, propagated to the pin range.
> >   * @pin_base: starting pin number of the bank.
> > @@ -170,6 +190,8 @@ struct samsung_pin_bank {
> >       enum eint_type  eint_type;
> >       u32             eint_mask;
> >       u32             eint_offset;
> > +     enum fltcon_type fltcon_type;
> > +     u32             fltcon_offset;
> >       const char      *name;
> >       u32             id;
> >
> > --
> > 2.43.0.rc2.451.g8631bc7472-goog
> >




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux