| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ac831e724393dd928d3e62a4cfe2d9d30af7a081.camel@linaro.org>
Date: Thu, 06 Mar 2025 15:26:28 +0000
From: André Draszik <andre.draszik@...aro.org>
To: Peter Griffin <peter.griffin@...aro.org>, Krzysztof Kozlowski
<krzk@...nel.org>, Sylwester Nawrocki <s.nawrocki@...sung.com>, Alim Akhtar
<alim.akhtar@...sung.com>, Linus Walleij <linus.walleij@...aro.org>
Cc: linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org,
linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
tudor.ambarus@...aro.org, willmcvicker@...gle.com,
semen.protsenko@...aro.org, kernel-team@...roid.com,
jaewon02.kim@...sung.com
Subject: Re: [PATCH v2 4/4] pinctrl: samsung: Add filter selection support
for alive bank on gs101
On Sat, 2025-03-01 at 11:43 +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@...aro.org>
> ---
>
> Changes since v1:
> * Remove eint_flt_selectable bool as it can be deduced from EINT_TYPE_WKUP (Peter)
> * Move filter config comment to header (Andre)
> * Rename EXYNOS_FLTCON_DELAY to EXYNOS_FLTCON_ANALOG (Andre)
> * Remove misleading old comment (Andre)
> * Refactor exynos_eint_update_flt_reg() into a loop (Andre)
>
> 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 | 35 ++++++++++++++++++++++++++++++++
> drivers/pinctrl/samsung/pinctrl-exynos.h | 21 +++++++++++++++++++
> 2 files changed, 56 insertions(+)
>
> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c
> index ddc7245ec2e5..4c467651b034 100644
> --- a/drivers/pinctrl/samsung/pinctrl-exynos.c
> +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c
> @@ -369,6 +369,39 @@ 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 |= con << shift;
EXYNOS_FLTCON_ANALOG == 0, so this code still needs to clear
bit 6 to undo a previous enabling of digital filter:
val &= ~(EXYNOS_FLTCON_DIGITAL << shift);
val |= con << shift;
> + }
> + writel(val, reg);
> +}
> +
> +/*
> + * Set the desired filter (digital or analog delay) and enable it 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_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;
> +
> + if (bank->eint_type != EINT_TYPE_WKUP)
> + return;
> +
> + 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.
> @@ -834,6 +867,7 @@ void gs101_pinctrl_suspend(struct samsung_pin_bank *bank)
> pr_debug("%s: save mask %#010x\n",
> bank->name, save->eint_mask);
> }
> + exynos_eint_set_filter(bank, EXYNOS_FLTCON_ANALOG);
Similar to patch 2, might be nicer to have this all if / else if to
make it more obvious that set_filter() is not unconditional.
> }
>
> void exynosautov920_pinctrl_suspend(struct samsung_pin_bank *bank)
> @@ -889,6 +923,7 @@ void gs101_pinctrl_resume(struct samsung_pin_bank *bank)
> writel(save->eint_mask, regs + bank->irq_chip->eint_mask
> + bank->eint_offset);
> }
> + exynos_eint_set_filter(bank, EXYNOS_FLTCON_DIGITAL);
dito.
Cheers,
Andre'
> }
>
> void exynos_pinctrl_resume(struct samsung_pin_bank *bank)
> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h b/drivers/pinctrl/samsung/pinctrl-exynos.h
> index 773f161a82a3..203d4b76a956 100644
> --- a/drivers/pinctrl/samsung/pinctrl-exynos.h
> +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h
> @@ -52,6 +52,27 @@
> #define EXYNOS_EINT_MAX_PER_BANK 8
> #define EXYNOS_EINT_NR_WKUP_EINT
>
> +/*
> + * EINT filter configuration 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 = Analog delay filter, 0x1 Digital filter (clock count)
> + * FLT_WIDTH Filtering width. Valid when FLT_SEL is 0x1
> + */
> +
> +#define EXYNOS_FLTCON_EN BIT(7)
> +#define EXYNOS_FLTCON_DIGITAL BIT(6)
> +#define EXYNOS_FLTCON_ANALOG (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, \
>
Powered by blists - more mailing lists