| Message ID | 20250120-pinctrl-fltcon-suspend-v1-3-e77900b2a854@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] pinctrl: samsung: add support for eint_fltcon_offset | expand |
Hi Peter, On Mon, 2025-01-20 at 22:34 +0000, Peter Griffin wrote: > Newer Exynos based SoCs have a filter selection bitfield in the filter > configuration registers on alive bank pins. This allows the selection of > a digital or analog delay filter for each pin. Add support for selecting > and enabling the filter. > > On suspend we set 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 > a clock and has an adjustable filter delay flt_width 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@linaro.org> > --- > > Note: this patch was previously sent as part of the initial gs101/ Pixel 6 > series and was dropped in v6. This new version incorporates the review > feedback from Sam Protsenko here in v5. > > Link: https://lore.kernel.org/all/20231201160925.3136868-1-peter.griffin@linaro.org/T/#m79ced98939e895c840d812c8b4c2b3f33ce604c8 > > Changes since previous version > * Drop fltcon_type enum and use bool eint_flt_selectable (Sam) > * Refactor and add exynos_eint_update_flt_reg() (Sam) > * Rename function to exynos_eint_set_filter() for easier readability (Sam) > * Remove comments and `if bank->fltcon_type != FLT_DEFAULT)` checks and indentation (Sam) > --- > drivers/pinctrl/samsung/pinctrl-exynos.c | 60 ++++++++++++++++++++++++++++++- > drivers/pinctrl/samsung/pinctrl-exynos.h | 9 +++++ > drivers/pinctrl/samsung/pinctrl-samsung.c | 1 + > drivers/pinctrl/samsung/pinctrl-samsung.h | 4 +++ > 4 files changed, 73 insertions(+), 1 deletion(-) > > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c > index ddc7245ec2e5..a0256715f8f6 100644 > --- a/drivers/pinctrl/samsung/pinctrl-exynos.c > +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c > @@ -369,6 +369,60 @@ struct exynos_eint_gpio_save { > u32 eint_mask; > }; > > +static void exynos_eint_update_flt_reg(void __iomem *reg, int cnt, int con) > +{ > + unsigned int val, shift; > + int i; > + > + val = readl(reg); > + for (i = 0; i < cnt; i++) { > + shift = i * EXYNOS_FLTCON_LEN; > + val &= ~(EXYNOS_FLTCON_MASK << shift); This is also clearing FLT_WIDTH. Is this intended? Should the value be retained / restored if the digital filter mode is selected? It seems in analog mode the width is ignored anyway, so maybe it doesn't need to be cleared? This might be more relevant if samsung-pinctrl implemented PIN_CONFIG_INPUT_DEBOUNCE (which it doesn't at the moment), but would still be good to allow that to work. > + val |= con << shift; > + } > + writel(val, reg); > +} > + > +/* > + * 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. > + * > + The FLTCON register (on alive banks) 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 It's a delay filter filter either way, right? If so, I think '0x0 = Delay filter' should instead be reworded to '0x0 = Analog filter'. > + * FLT_WIDTH Filtering width. Valid when FLT_SEL is 0x1 This complete above register layout description would be better suited right above the macro definition in pinctrl-exynos.h I believe. > + */ > +static void exynos_eint_set_filter(struct samsung_pin_bank *bank, int filter) > +{ > + unsigned int off = EXYNOS_GPIO_EFLTCON_OFFSET + bank->eint_fltcon_offset; > + void __iomem *reg = bank->drvdata->virt_base + off; > + unsigned int con = EXYNOS_FLTCON_EN | filter; > + u8 n = bank->nr_pins; > + > + if (!bank->eint_flt_selectable) > + return; > + > + /* > + * If nr_pins > 4, we should set FLTCON0 register fully (pin0~3). > + * So loop 4 times in case of FLTCON0. Loop for FLTCON1 pin4~7. > + */ This comment is a bit confusing now. There is no loop left here (as it has moved). The loop is an implementation detail of exynos_eint_update_flt_reg() and shouldn't be referred to here. > + if (n <= 4) { > + exynos_eint_update_flt_reg(reg, n, con); > + } else { > + exynos_eint_update_flt_reg(reg, 4, con); > + exynos_eint_update_flt_reg(reg + 0x4, n - 4, con); > + } How about something like this instead of if/else: for (int n = 0; n < bank->nr_pins; n += 4) exynos_eint_update_flt_reg(reg + n, min(bank->nr_pins - n, 4), con); > +} > + > /* > * exynos_eint_gpio_init() - setup handling of external gpio interrupts. > * @d: driver data of samsung pinctrl driver. > @@ -420,7 +474,7 @@ __init int exynos_eint_gpio_init(struct samsung_pinctrl_drv_data *d) > ret = -ENOMEM; > goto err_domains; > } > - > + exynos_eint_set_filter(bank, EXYNOS_FLTCON_DELAY); > } > > return 0; > @@ -833,6 +887,8 @@ void gs101_pinctrl_suspend(struct samsung_pin_bank *bank) > bank->name, save->eint_fltcon1); > pr_debug("%s: save mask %#010x\n", > bank->name, save->eint_mask); > + } else if (bank->eint_type == EINT_TYPE_WKUP) { > + exynos_eint_set_filter(bank, EXYNOS_FLTCON_DELAY); > } > } > > @@ -888,6 +944,8 @@ void gs101_pinctrl_resume(struct samsung_pin_bank *bank) > writel(save->eint_fltcon1, eint_fltcfg0 + 4); > writel(save->eint_mask, regs + bank->irq_chip->eint_mask > + bank->eint_offset); > + } else if (bank->eint_type == EINT_TYPE_WKUP) { > + exynos_eint_set_filter(bank, EXYNOS_FLTCON_DIGITAL); > } > } > > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h b/drivers/pinctrl/samsung/pinctrl-exynos.h > index 773f161a82a3..4f2dc6a2e5c7 100644 > --- a/drivers/pinctrl/samsung/pinctrl-exynos.h > +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h > @@ -52,6 +52,13 @@ > #define EXYNOS_EINT_MAX_PER_BANK 8 > #define EXYNOS_EINT_NR_WKUP_EINT > > +/* EINT filter configuration */ > +#define EXYNOS_FLTCON_EN BIT(7) > +#define EXYNOS_FLTCON_DIGITAL BIT(6) > +#define EXYNOS_FLTCON_DELAY (0 << 6) should EXYNOS_FLTCON_DELAY be EXYNOS_FLTCON_ANALOG? Cheers, Andre' > +#define EXYNOS_FLTCON_MASK GENMASK(7, 0) > +#define EXYNOS_FLTCON_LEN 8 > + > #define EXYNOS_PIN_BANK_EINTN(pins, reg, id) \ > { \ > .type = &bank_type_off, \ > @@ -183,6 +190,7 @@ > .eint_type = EINT_TYPE_GPIO, \ > .eint_offset = offs, \ > .eint_fltcon_offset = fltcon_offs, \ > + .eint_flt_selectable = false, \ > .name = id \ > } > > @@ -194,6 +202,7 @@ > .eint_type = EINT_TYPE_WKUP, \ > .eint_offset = offs, \ > .eint_fltcon_offset = fltcon_offs, \ > + .eint_flt_selectable = true, \ > .name = id \ > } > > diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c > index 375634d8cc79..9b874ab2c89b 100644 > --- a/drivers/pinctrl/samsung/pinctrl-samsung.c > +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c > @@ -1231,6 +1231,7 @@ samsung_pinctrl_get_soc_data(struct samsung_pinctrl_drv_data *d, > bank->eint_mask_offset = bdata->eint_mask_offset; > bank->eint_pend_offset = bdata->eint_pend_offset; > bank->eint_fltcon_offset = bdata->eint_fltcon_offset; > + bank->eint_flt_selectable = bdata->eint_flt_selectable; > bank->name = bdata->name; > > raw_spin_lock_init(&bank->slock); > diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h b/drivers/pinctrl/samsung/pinctrl-samsung.h > index e939e5bb0347..22f3c1e15e6a 100644 > --- a/drivers/pinctrl/samsung/pinctrl-samsung.h > +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h > @@ -145,6 +145,7 @@ struct samsung_pin_bank_type { > * @eint_mask_offset: ExynosAuto SoC-specific EINT mask register offset of bank. > * @eint_pend_offset: ExynosAuto SoC-specific EINT pend register offset of bank. > * @eint_fltcon_offset: GS101 SoC-specific EINT filter config register offset. > + * @eint_flt_selectable: whether the filter (delay/digital) is selectable. > * @name: name to be prefixed for each pin in this pin bank. > */ > struct samsung_pin_bank_data { > @@ -160,6 +161,7 @@ struct samsung_pin_bank_data { > u32 eint_mask_offset; > u32 eint_pend_offset; > u32 eint_fltcon_offset; > + bool eint_flt_selectable; > const char *name; > }; > > @@ -178,6 +180,7 @@ struct samsung_pin_bank_data { > * @eint_mask_offset: ExynosAuto SoC-specific EINT mask register offset of bank. > * @eint_pend_offset: ExynosAuto SoC-specific EINT pend register offset of bank. > * @eint_fltcon_offset: GS101 SoC-specific EINT filter config register offset. > + * @eint_flt_selectable: whether the filter (delay/digital) is selectable > * @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. > @@ -205,6 +208,7 @@ struct samsung_pin_bank { > u32 eint_mask_offset; > u32 eint_pend_offset; > u32 eint_fltcon_offset; > + bool eint_flt_selectable; > const char *name; > u32 id; > >
Hi André, Thanks for your review feedback. On Tue, 21 Jan 2025 at 11:04, André Draszik <andre.draszik@linaro.org> wrote: > > Hi Peter, > > On Mon, 2025-01-20 at 22:34 +0000, Peter Griffin wrote: > > Newer Exynos based SoCs have a filter selection bitfield in the filter > > configuration registers on alive bank pins. This allows the selection of > > a digital or analog delay filter for each pin. Add support for selecting > > and enabling the filter. > > > > On suspend we set 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 > > a clock and has an adjustable filter delay flt_width 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@linaro.org> > > --- > > > > Note: this patch was previously sent as part of the initial gs101/ Pixel 6 > > series and was dropped in v6. This new version incorporates the review > > feedback from Sam Protsenko here in v5. > > > > Link: https://lore.kernel.org/all/20231201160925.3136868-1-peter.griffin@linaro.org/T/#m79ced98939e895c840d812c8b4c2b3f33ce604c8 > > > > Changes since previous version > > * Drop fltcon_type enum and use bool eint_flt_selectable (Sam) > > * Refactor and add exynos_eint_update_flt_reg() (Sam) > > * Rename function to exynos_eint_set_filter() for easier readability (Sam) > > * Remove comments and `if bank->fltcon_type != FLT_DEFAULT)` checks and indentation (Sam) > > --- > > drivers/pinctrl/samsung/pinctrl-exynos.c | 60 ++++++++++++++++++++++++++++++- > > drivers/pinctrl/samsung/pinctrl-exynos.h | 9 +++++ > > drivers/pinctrl/samsung/pinctrl-samsung.c | 1 + > > drivers/pinctrl/samsung/pinctrl-samsung.h | 4 +++ > > 4 files changed, 73 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c > > index ddc7245ec2e5..a0256715f8f6 100644 > > --- a/drivers/pinctrl/samsung/pinctrl-exynos.c > > +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c > > @@ -369,6 +369,60 @@ struct exynos_eint_gpio_save { > > u32 eint_mask; > > }; > > > > +static void exynos_eint_update_flt_reg(void __iomem *reg, int cnt, int con) > > +{ > > + unsigned int val, shift; > > + int i; > > + > > + val = readl(reg); > > + for (i = 0; i < cnt; i++) { > > + shift = i * EXYNOS_FLTCON_LEN; > > + val &= ~(EXYNOS_FLTCON_MASK << shift); > > This is also clearing FLT_WIDTH. Is this intended? Should the > value be retained / restored if the digital filter mode is > selected? It seems in analog mode the width is ignored anyway, > so maybe it doesn't need to be cleared? Currently we don't support setting the FLT_WIDTH bitfield and the reset value is zero. But I guess it would be better to not clear the bitfield in case the bootloader does set a value in the future. I'll update to do that in the next version. > This might be more relevant if samsung-pinctrl implemented > PIN_CONFIG_INPUT_DEBOUNCE (which it doesn't at the moment), > but would still be good to allow that to work. > > > + val |= con << shift; > > + } > > + writel(val, reg); > > +} > > + > > +/* > > + * 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. > > + * > > + The FLTCON register (on alive banks) 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 > > It's a delay filter filter either way, right? If so, I > think '0x0 = Delay filter' should instead be reworded to > '0x0 = Analog filter'. I see your point, and kind of agree that Analog is a better name. The rationale for going with "Digital filter" and "Delay filter" was that it matches the FLT_SEL bitfield description in the datasheet. I thought it might confuse people using a different name. The info about it being Analog filter came via a bug from Samsung. But if folks prefer Analog I can use that instead. @Krzysztof any thoughts on the above naming? > > > + * FLT_WIDTH Filtering width. Valid when FLT_SEL is 0x1 > > This complete above register layout description would be > better suited right above the macro definition in > pinctrl-exynos.h I believe. I can move the comment in the next version. > > > + */ > > +static void exynos_eint_set_filter(struct samsung_pin_bank *bank, int filter) > > +{ > > + unsigned int off = EXYNOS_GPIO_EFLTCON_OFFSET + bank->eint_fltcon_offset; > > + void __iomem *reg = bank->drvdata->virt_base + off; > > + unsigned int con = EXYNOS_FLTCON_EN | filter; > > + u8 n = bank->nr_pins; > > + > > + if (!bank->eint_flt_selectable) > > + return; > > + > > + /* > > + * If nr_pins > 4, we should set FLTCON0 register fully (pin0~3). > > + * So loop 4 times in case of FLTCON0. Loop for FLTCON1 pin4~7. > > + */ > > This comment is a bit confusing now. There is no loop left here (as > it has moved). The loop is an implementation detail of > exynos_eint_update_flt_reg() and shouldn't be referred to here. Will fix. > > > + if (n <= 4) { > > + exynos_eint_update_flt_reg(reg, n, con); > > + } else { > > + exynos_eint_update_flt_reg(reg, 4, con); > > + exynos_eint_update_flt_reg(reg + 0x4, n - 4, con); > > + } > > How about something like this instead of if/else: > > for (int n = 0; n < bank->nr_pins; n += 4) > exynos_eint_update_flt_reg(reg + n, > min(bank->nr_pins - n, 4), con); Will update as you suggest. > > > > +} > > + > > /* > > * exynos_eint_gpio_init() - setup handling of external gpio interrupts. > > * @d: driver data of samsung pinctrl driver. > > @@ -420,7 +474,7 @@ __init int exynos_eint_gpio_init(struct samsung_pinctrl_drv_data *d) > > ret = -ENOMEM; > > goto err_domains; > > } > > - > > + exynos_eint_set_filter(bank, EXYNOS_FLTCON_DELAY); > > } > > > > return 0; > > @@ -833,6 +887,8 @@ void gs101_pinctrl_suspend(struct samsung_pin_bank *bank) > > bank->name, save->eint_fltcon1); > > pr_debug("%s: save mask %#010x\n", > > bank->name, save->eint_mask); > > + } else if (bank->eint_type == EINT_TYPE_WKUP) { > > + exynos_eint_set_filter(bank, EXYNOS_FLTCON_DELAY); > > } > > } > > > > @@ -888,6 +944,8 @@ void gs101_pinctrl_resume(struct samsung_pin_bank *bank) > > writel(save->eint_fltcon1, eint_fltcfg0 + 4); > > writel(save->eint_mask, regs + bank->irq_chip->eint_mask > > + bank->eint_offset); > > + } else if (bank->eint_type == EINT_TYPE_WKUP) { > > + exynos_eint_set_filter(bank, EXYNOS_FLTCON_DIGITAL); > > } > > } > > > > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h b/drivers/pinctrl/samsung/pinctrl-exynos.h > > index 773f161a82a3..4f2dc6a2e5c7 100644 > > --- a/drivers/pinctrl/samsung/pinctrl-exynos.h > > +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h > > @@ -52,6 +52,13 @@ > > #define EXYNOS_EINT_MAX_PER_BANK 8 > > #define EXYNOS_EINT_NR_WKUP_EINT > > > > +/* EINT filter configuration */ > > +#define EXYNOS_FLTCON_EN BIT(7) > > +#define EXYNOS_FLTCON_DIGITAL BIT(6) > > +#define EXYNOS_FLTCON_DELAY (0 << 6) > > should EXYNOS_FLTCON_DELAY be EXYNOS_FLTCON_ANALOG? Same comment as above, I used DELAY because it matches the datasheet bitfield description but I agree that ANALOG is a better name and would make code readability better. The downside is we don't match the datasheet description. Thanks, Peter
diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c index ddc7245ec2e5..a0256715f8f6 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c @@ -369,6 +369,60 @@ struct exynos_eint_gpio_save { u32 eint_mask; }; +static void exynos_eint_update_flt_reg(void __iomem *reg, int cnt, int con) +{ + unsigned int val, shift; + int i; + + val = readl(reg); + for (i = 0; i < cnt; i++) { + shift = i * EXYNOS_FLTCON_LEN; + val &= ~(EXYNOS_FLTCON_MASK << shift); + val |= con << shift; + } + writel(val, reg); +} + +/* + * 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. + * + The FLTCON register (on alive banks) 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 + */ +static void exynos_eint_set_filter(struct samsung_pin_bank *bank, int filter) +{ + unsigned int off = EXYNOS_GPIO_EFLTCON_OFFSET + bank->eint_fltcon_offset; + void __iomem *reg = bank->drvdata->virt_base + off; + unsigned int con = EXYNOS_FLTCON_EN | filter; + u8 n = bank->nr_pins; + + if (!bank->eint_flt_selectable) + return; + + /* + * If nr_pins > 4, we should set FLTCON0 register fully (pin0~3). + * So loop 4 times in case of FLTCON0. Loop for FLTCON1 pin4~7. + */ + if (n <= 4) { + exynos_eint_update_flt_reg(reg, n, con); + } else { + exynos_eint_update_flt_reg(reg, 4, con); + exynos_eint_update_flt_reg(reg + 0x4, n - 4, con); + } +} + /* * exynos_eint_gpio_init() - setup handling of external gpio interrupts. * @d: driver data of samsung pinctrl driver. @@ -420,7 +474,7 @@ __init int exynos_eint_gpio_init(struct samsung_pinctrl_drv_data *d) ret = -ENOMEM; goto err_domains; } - + exynos_eint_set_filter(bank, EXYNOS_FLTCON_DELAY); } return 0; @@ -833,6 +887,8 @@ void gs101_pinctrl_suspend(struct samsung_pin_bank *bank) bank->name, save->eint_fltcon1); pr_debug("%s: save mask %#010x\n", bank->name, save->eint_mask); + } else if (bank->eint_type == EINT_TYPE_WKUP) { + exynos_eint_set_filter(bank, EXYNOS_FLTCON_DELAY); } } @@ -888,6 +944,8 @@ void gs101_pinctrl_resume(struct samsung_pin_bank *bank) writel(save->eint_fltcon1, eint_fltcfg0 + 4); writel(save->eint_mask, regs + bank->irq_chip->eint_mask + bank->eint_offset); + } else if (bank->eint_type == EINT_TYPE_WKUP) { + exynos_eint_set_filter(bank, EXYNOS_FLTCON_DIGITAL); } } diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h b/drivers/pinctrl/samsung/pinctrl-exynos.h index 773f161a82a3..4f2dc6a2e5c7 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.h +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h @@ -52,6 +52,13 @@ #define EXYNOS_EINT_MAX_PER_BANK 8 #define EXYNOS_EINT_NR_WKUP_EINT +/* EINT filter configuration */ +#define EXYNOS_FLTCON_EN BIT(7) +#define EXYNOS_FLTCON_DIGITAL BIT(6) +#define EXYNOS_FLTCON_DELAY (0 << 6) +#define EXYNOS_FLTCON_MASK GENMASK(7, 0) +#define EXYNOS_FLTCON_LEN 8 + #define EXYNOS_PIN_BANK_EINTN(pins, reg, id) \ { \ .type = &bank_type_off, \ @@ -183,6 +190,7 @@ .eint_type = EINT_TYPE_GPIO, \ .eint_offset = offs, \ .eint_fltcon_offset = fltcon_offs, \ + .eint_flt_selectable = false, \ .name = id \ } @@ -194,6 +202,7 @@ .eint_type = EINT_TYPE_WKUP, \ .eint_offset = offs, \ .eint_fltcon_offset = fltcon_offs, \ + .eint_flt_selectable = true, \ .name = id \ } diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c index 375634d8cc79..9b874ab2c89b 100644 --- a/drivers/pinctrl/samsung/pinctrl-samsung.c +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c @@ -1231,6 +1231,7 @@ samsung_pinctrl_get_soc_data(struct samsung_pinctrl_drv_data *d, bank->eint_mask_offset = bdata->eint_mask_offset; bank->eint_pend_offset = bdata->eint_pend_offset; bank->eint_fltcon_offset = bdata->eint_fltcon_offset; + bank->eint_flt_selectable = bdata->eint_flt_selectable; bank->name = bdata->name; raw_spin_lock_init(&bank->slock); diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h b/drivers/pinctrl/samsung/pinctrl-samsung.h index e939e5bb0347..22f3c1e15e6a 100644 --- a/drivers/pinctrl/samsung/pinctrl-samsung.h +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h @@ -145,6 +145,7 @@ struct samsung_pin_bank_type { * @eint_mask_offset: ExynosAuto SoC-specific EINT mask register offset of bank. * @eint_pend_offset: ExynosAuto SoC-specific EINT pend register offset of bank. * @eint_fltcon_offset: GS101 SoC-specific EINT filter config register offset. + * @eint_flt_selectable: whether the filter (delay/digital) is selectable. * @name: name to be prefixed for each pin in this pin bank. */ struct samsung_pin_bank_data { @@ -160,6 +161,7 @@ struct samsung_pin_bank_data { u32 eint_mask_offset; u32 eint_pend_offset; u32 eint_fltcon_offset; + bool eint_flt_selectable; const char *name; }; @@ -178,6 +180,7 @@ struct samsung_pin_bank_data { * @eint_mask_offset: ExynosAuto SoC-specific EINT mask register offset of bank. * @eint_pend_offset: ExynosAuto SoC-specific EINT pend register offset of bank. * @eint_fltcon_offset: GS101 SoC-specific EINT filter config register offset. + * @eint_flt_selectable: whether the filter (delay/digital) is selectable * @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. @@ -205,6 +208,7 @@ struct samsung_pin_bank { u32 eint_mask_offset; u32 eint_pend_offset; u32 eint_fltcon_offset; + bool eint_flt_selectable; const char *name; u32 id;
Newer Exynos based SoCs have a filter selection bitfield in the filter configuration registers on alive bank pins. This allows the selection of a digital or analog delay filter for each pin. Add support for selecting and enabling the filter. On suspend we set 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 a clock and has an adjustable filter delay flt_width 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@linaro.org> --- Note: this patch was previously sent as part of the initial gs101/ Pixel 6 series and was dropped in v6. This new version incorporates the review feedback from Sam Protsenko here in v5. Link: https://lore.kernel.org/all/20231201160925.3136868-1-peter.griffin@linaro.org/T/#m79ced98939e895c840d812c8b4c2b3f33ce604c8 Changes since previous version * Drop fltcon_type enum and use bool eint_flt_selectable (Sam) * Refactor and add exynos_eint_update_flt_reg() (Sam) * Rename function to exynos_eint_set_filter() for easier readability (Sam) * Remove comments and `if bank->fltcon_type != FLT_DEFAULT)` checks and indentation (Sam) --- drivers/pinctrl/samsung/pinctrl-exynos.c | 60 ++++++++++++++++++++++++++++++- drivers/pinctrl/samsung/pinctrl-exynos.h | 9 +++++ drivers/pinctrl/samsung/pinctrl-samsung.c | 1 + drivers/pinctrl/samsung/pinctrl-samsung.h | 4 +++ 4 files changed, 73 insertions(+), 1 deletion(-)