* [PATCH v2 0/4] samsung: pinctrl: Add support for eint_fltcon_offset and filter selection on gs101
@ 2025-03-01 11:43 Peter Griffin
2025-03-01 11:43 ` [PATCH v2 1/4] pinctrl: samsung: add support for eint_fltcon_offset Peter Griffin
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Peter Griffin @ 2025-03-01 11:43 UTC (permalink / raw)
To: Krzysztof Kozlowski, Sylwester Nawrocki, Alim Akhtar,
Linus Walleij
Cc: linux-arm-kernel, linux-samsung-soc, linux-gpio, linux-kernel,
andre.draszik, tudor.ambarus, willmcvicker, semen.protsenko,
kernel-team, jaewon02.kim, Peter Griffin, stable
Hi folks,
This series fixes support for correctly saving and restoring fltcon0
and fltcon1 registers on gs101 for non-alive banks where the fltcon
register offset is not at a fixed offset (unlike previous SoCs).
This is done by adding a eint_fltcon_offset and providing GS101
specific pin macros that take an additional parameter (similar to
how exynosautov920 handles it's eint_con_offset).
Additionally the SoC specific suspend and resume callbacks are
re-factored so that each SoC variant has it's own callback containing
the peculiarities for that SoC.
Finally support for filter selection on alive banks is added, this is
currently only enabled for gs101. The code path can be excercised using
`echo mem > /sys/power/state`
regards,
Peter
To: Krzysztof Kozlowski <krzk@kernel.org>
To: Sylwester Nawrocki <s.nawrocki@samsung.com>
To: Alim Akhtar <alim.akhtar@samsung.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: linux-gpio@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: andre.draszik@linaro.org
Cc: tudor.ambarus@linaro.org
Cc: willmcvicker@google.com
Cc: semen.protsenko@linaro.org
Cc: kernel-team@android.com
Cc: jaewon02.kim@samsung.com
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
Changes in v2:
- Remove eint_flt_selectable bool as it can be deduced from EINT_TYPE_WKUP (Peter)
- Move filter config register comment to header file (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)
- Split refactor of suspend/resume callbacks & gs101 parts into separate patches (Andre)
- Link to v1: https://lore.kernel.org/r/20250120-pinctrl-fltcon-suspend-v1-0-e77900b2a854@linaro.org
---
Peter Griffin (4):
pinctrl: samsung: add support for eint_fltcon_offset
pinctrl: samsung: add dedicated SoC eint suspend/resume callbacks
pinctrl: samsung: add gs101 specific eint suspend/resume callbacks
pinctrl: samsung: Add filter selection support for alive bank on gs101
drivers/pinctrl/samsung/pinctrl-exynos-arm64.c | 150 ++++++-------
drivers/pinctrl/samsung/pinctrl-exynos.c | 293 +++++++++++++++----------
drivers/pinctrl/samsung/pinctrl-exynos.h | 51 ++++-
drivers/pinctrl/samsung/pinctrl-samsung.c | 12 +-
drivers/pinctrl/samsung/pinctrl-samsung.h | 12 +-
5 files changed, 321 insertions(+), 197 deletions(-)
---
base-commit: f7da3699c901aea6a009d38116d24c67a4c9662e
change-id: 20250120-pinctrl-fltcon-suspend-2333a137c4d4
Best regards,
--
Peter Griffin <peter.griffin@linaro.org>
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v2 1/4] pinctrl: samsung: add support for eint_fltcon_offset 2025-03-01 11:43 [PATCH v2 0/4] samsung: pinctrl: Add support for eint_fltcon_offset and filter selection on gs101 Peter Griffin @ 2025-03-01 11:43 ` Peter Griffin 2025-03-06 11:24 ` André Draszik 2025-03-01 11:43 ` [PATCH v2 2/4] pinctrl: samsung: add dedicated SoC eint suspend/resume callbacks Peter Griffin ` (2 subsequent siblings) 3 siblings, 1 reply; 11+ messages in thread From: Peter Griffin @ 2025-03-01 11:43 UTC (permalink / raw) To: Krzysztof Kozlowski, Sylwester Nawrocki, Alim Akhtar, Linus Walleij Cc: linux-arm-kernel, linux-samsung-soc, linux-gpio, linux-kernel, andre.draszik, tudor.ambarus, willmcvicker, semen.protsenko, kernel-team, jaewon02.kim, Peter Griffin, stable On gs101 SoC the fltcon0 (filter configuration 0) offset isn't at a fixed offset like previous SoCs as the fltcon1 register only exists when there are more than 4 pins in the bank. Add a eint_fltcon_offset and new GS101_PIN_BANK_EINT* macros that take an additional fltcon_offs variable. This can then be used in suspend/resume callbacks to save and restore the fltcon0 and fltcon1 registers. Signed-off-by: Peter Griffin <peter.griffin@linaro.org> Fixes: 4a8be01a1a7a ("pinctrl: samsung: Add gs101 SoC pinctrl configuration") Cc: stable@vger.kernel.org --- drivers/pinctrl/samsung/pinctrl-exynos-arm64.c | 98 +++++++++++++------------- drivers/pinctrl/samsung/pinctrl-exynos.h | 22 ++++++ drivers/pinctrl/samsung/pinctrl-samsung.c | 1 + drivers/pinctrl/samsung/pinctrl-samsung.h | 4 ++ 4 files changed, 76 insertions(+), 49 deletions(-) diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c index 3ea7106ce5ea..e28fe8177646 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c @@ -1370,83 +1370,83 @@ const struct samsung_pinctrl_of_match_data fsd_of_data __initconst = { /* pin banks of gs101 pin-controller (ALIVE) */ static const struct samsung_pin_bank_data gs101_pin_alive[] = { - EXYNOS850_PIN_BANK_EINTW(8, 0x0, "gpa0", 0x00), - EXYNOS850_PIN_BANK_EINTW(7, 0x20, "gpa1", 0x04), - EXYNOS850_PIN_BANK_EINTW(5, 0x40, "gpa2", 0x08), - EXYNOS850_PIN_BANK_EINTW(4, 0x60, "gpa3", 0x0c), - EXYNOS850_PIN_BANK_EINTW(4, 0x80, "gpa4", 0x10), - EXYNOS850_PIN_BANK_EINTW(7, 0xa0, "gpa5", 0x14), - EXYNOS850_PIN_BANK_EINTW(8, 0xc0, "gpa9", 0x18), - EXYNOS850_PIN_BANK_EINTW(2, 0xe0, "gpa10", 0x1c), + GS101_PIN_BANK_EINTW(8, 0x0, "gpa0", 0x00, 0x00), + GS101_PIN_BANK_EINTW(7, 0x20, "gpa1", 0x04, 0x08), + GS101_PIN_BANK_EINTW(5, 0x40, "gpa2", 0x08, 0x10), + GS101_PIN_BANK_EINTW(4, 0x60, "gpa3", 0x0c, 0x18), + GS101_PIN_BANK_EINTW(4, 0x80, "gpa4", 0x10, 0x1c), + GS101_PIN_BANK_EINTW(7, 0xa0, "gpa5", 0x14, 0x20), + GS101_PIN_BANK_EINTW(8, 0xc0, "gpa9", 0x18, 0x28), + GS101_PIN_BANK_EINTW(2, 0xe0, "gpa10", 0x1c, 0x30), }; /* pin banks of gs101 pin-controller (FAR_ALIVE) */ static const struct samsung_pin_bank_data gs101_pin_far_alive[] = { - EXYNOS850_PIN_BANK_EINTW(8, 0x0, "gpa6", 0x00), - EXYNOS850_PIN_BANK_EINTW(4, 0x20, "gpa7", 0x04), - EXYNOS850_PIN_BANK_EINTW(8, 0x40, "gpa8", 0x08), - EXYNOS850_PIN_BANK_EINTW(2, 0x60, "gpa11", 0x0c), + GS101_PIN_BANK_EINTW(8, 0x0, "gpa6", 0x00, 0x00), + GS101_PIN_BANK_EINTW(4, 0x20, "gpa7", 0x04, 0x08), + GS101_PIN_BANK_EINTW(8, 0x40, "gpa8", 0x08, 0x0c), + GS101_PIN_BANK_EINTW(2, 0x60, "gpa11", 0x0c, 0x14), }; /* pin banks of gs101 pin-controller (GSACORE) */ static const struct samsung_pin_bank_data gs101_pin_gsacore[] = { - EXYNOS850_PIN_BANK_EINTG(2, 0x0, "gps0", 0x00), - EXYNOS850_PIN_BANK_EINTG(8, 0x20, "gps1", 0x04), - EXYNOS850_PIN_BANK_EINTG(3, 0x40, "gps2", 0x08), + GS101_PIN_BANK_EINTG(2, 0x0, "gps0", 0x00, 0x00), + GS101_PIN_BANK_EINTG(8, 0x20, "gps1", 0x04, 0x04), + GS101_PIN_BANK_EINTG(3, 0x40, "gps2", 0x08, 0x0c), }; /* pin banks of gs101 pin-controller (GSACTRL) */ static const struct samsung_pin_bank_data gs101_pin_gsactrl[] = { - EXYNOS850_PIN_BANK_EINTW(6, 0x0, "gps3", 0x00), + GS101_PIN_BANK_EINTW(6, 0x0, "gps3", 0x00, 0x00), }; /* pin banks of gs101 pin-controller (PERIC0) */ static const struct samsung_pin_bank_data gs101_pin_peric0[] = { - EXYNOS850_PIN_BANK_EINTG(5, 0x0, "gpp0", 0x00), - EXYNOS850_PIN_BANK_EINTG(4, 0x20, "gpp1", 0x04), - EXYNOS850_PIN_BANK_EINTG(4, 0x40, "gpp2", 0x08), - EXYNOS850_PIN_BANK_EINTG(2, 0x60, "gpp3", 0x0c), - EXYNOS850_PIN_BANK_EINTG(4, 0x80, "gpp4", 0x10), - EXYNOS850_PIN_BANK_EINTG(2, 0xa0, "gpp5", 0x14), - EXYNOS850_PIN_BANK_EINTG(4, 0xc0, "gpp6", 0x18), - EXYNOS850_PIN_BANK_EINTG(2, 0xe0, "gpp7", 0x1c), - EXYNOS850_PIN_BANK_EINTG(4, 0x100, "gpp8", 0x20), - EXYNOS850_PIN_BANK_EINTG(2, 0x120, "gpp9", 0x24), - EXYNOS850_PIN_BANK_EINTG(4, 0x140, "gpp10", 0x28), - EXYNOS850_PIN_BANK_EINTG(2, 0x160, "gpp11", 0x2c), - EXYNOS850_PIN_BANK_EINTG(4, 0x180, "gpp12", 0x30), - EXYNOS850_PIN_BANK_EINTG(2, 0x1a0, "gpp13", 0x34), - EXYNOS850_PIN_BANK_EINTG(4, 0x1c0, "gpp14", 0x38), - EXYNOS850_PIN_BANK_EINTG(2, 0x1e0, "gpp15", 0x3c), - EXYNOS850_PIN_BANK_EINTG(4, 0x200, "gpp16", 0x40), - EXYNOS850_PIN_BANK_EINTG(2, 0x220, "gpp17", 0x44), - EXYNOS850_PIN_BANK_EINTG(4, 0x240, "gpp18", 0x48), - EXYNOS850_PIN_BANK_EINTG(4, 0x260, "gpp19", 0x4c), + GS101_PIN_BANK_EINTG(5, 0x0, "gpp0", 0x00, 0x00), + GS101_PIN_BANK_EINTG(4, 0x20, "gpp1", 0x04, 0x08), + GS101_PIN_BANK_EINTG(4, 0x40, "gpp2", 0x08, 0x0c), + GS101_PIN_BANK_EINTG(2, 0x60, "gpp3", 0x0c, 0x10), + GS101_PIN_BANK_EINTG(4, 0x80, "gpp4", 0x10, 0x14), + GS101_PIN_BANK_EINTG(2, 0xa0, "gpp5", 0x14, 0x18), + GS101_PIN_BANK_EINTG(4, 0xc0, "gpp6", 0x18, 0x1c), + GS101_PIN_BANK_EINTG(2, 0xe0, "gpp7", 0x1c, 0x20), + GS101_PIN_BANK_EINTG(4, 0x100, "gpp8", 0x20, 0x24), + GS101_PIN_BANK_EINTG(2, 0x120, "gpp9", 0x24, 0x28), + GS101_PIN_BANK_EINTG(4, 0x140, "gpp10", 0x28, 0x2c), + GS101_PIN_BANK_EINTG(2, 0x160, "gpp11", 0x2c, 0x30), + GS101_PIN_BANK_EINTG(4, 0x180, "gpp12", 0x30, 0x34), + GS101_PIN_BANK_EINTG(2, 0x1a0, "gpp13", 0x34, 0x38), + GS101_PIN_BANK_EINTG(4, 0x1c0, "gpp14", 0x38, 0x3c), + GS101_PIN_BANK_EINTG(2, 0x1e0, "gpp15", 0x3c, 0x40), + GS101_PIN_BANK_EINTG(4, 0x200, "gpp16", 0x40, 0x44), + GS101_PIN_BANK_EINTG(2, 0x220, "gpp17", 0x44, 0x48), + GS101_PIN_BANK_EINTG(4, 0x240, "gpp18", 0x48, 0x4c), + GS101_PIN_BANK_EINTG(4, 0x260, "gpp19", 0x4c, 0x50), }; /* pin banks of gs101 pin-controller (PERIC1) */ static const struct samsung_pin_bank_data gs101_pin_peric1[] = { - EXYNOS850_PIN_BANK_EINTG(8, 0x0, "gpp20", 0x00), - EXYNOS850_PIN_BANK_EINTG(4, 0x20, "gpp21", 0x04), - EXYNOS850_PIN_BANK_EINTG(2, 0x40, "gpp22", 0x08), - EXYNOS850_PIN_BANK_EINTG(8, 0x60, "gpp23", 0x0c), - EXYNOS850_PIN_BANK_EINTG(4, 0x80, "gpp24", 0x10), - EXYNOS850_PIN_BANK_EINTG(4, 0xa0, "gpp25", 0x14), - EXYNOS850_PIN_BANK_EINTG(5, 0xc0, "gpp26", 0x18), - EXYNOS850_PIN_BANK_EINTG(4, 0xe0, "gpp27", 0x1c), + GS101_PIN_BANK_EINTG(8, 0x0, "gpp20", 0x00, 0x00), + GS101_PIN_BANK_EINTG(4, 0x20, "gpp21", 0x04, 0x08), + GS101_PIN_BANK_EINTG(2, 0x40, "gpp22", 0x08, 0x0c), + GS101_PIN_BANK_EINTG(8, 0x60, "gpp23", 0x0c, 0x10), + GS101_PIN_BANK_EINTG(4, 0x80, "gpp24", 0x10, 0x18), + GS101_PIN_BANK_EINTG(4, 0xa0, "gpp25", 0x14, 0x1c), + GS101_PIN_BANK_EINTG(5, 0xc0, "gpp26", 0x18, 0x20), + GS101_PIN_BANK_EINTG(4, 0xe0, "gpp27", 0x1c, 0x28), }; /* pin banks of gs101 pin-controller (HSI1) */ static const struct samsung_pin_bank_data gs101_pin_hsi1[] = { - EXYNOS850_PIN_BANK_EINTG(6, 0x0, "gph0", 0x00), - EXYNOS850_PIN_BANK_EINTG(7, 0x20, "gph1", 0x04), + GS101_PIN_BANK_EINTG(6, 0x0, "gph0", 0x00, 0x00), + GS101_PIN_BANK_EINTG(7, 0x20, "gph1", 0x04, 0x08), }; /* pin banks of gs101 pin-controller (HSI2) */ static const struct samsung_pin_bank_data gs101_pin_hsi2[] = { - EXYNOS850_PIN_BANK_EINTG(6, 0x0, "gph2", 0x00), - EXYNOS850_PIN_BANK_EINTG(2, 0x20, "gph3", 0x04), - EXYNOS850_PIN_BANK_EINTG(6, 0x40, "gph4", 0x08), + GS101_PIN_BANK_EINTG(6, 0x0, "gph2", 0x00, 0x00), + GS101_PIN_BANK_EINTG(2, 0x20, "gph3", 0x04, 0x08), + GS101_PIN_BANK_EINTG(6, 0x40, "gph4", 0x08, 0x0c), }; static const struct samsung_pin_ctrl gs101_pin_ctrl[] __initconst = { diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h b/drivers/pinctrl/samsung/pinctrl-exynos.h index 7b7ff7ffeb56..33df21d5c9d6 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.h +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h @@ -175,6 +175,28 @@ .name = id \ } +#define GS101_PIN_BANK_EINTG(pins, reg, id, offs, fltcon_offs) \ + { \ + .type = &exynos850_bank_type_off, \ + .pctl_offset = reg, \ + .nr_pins = pins, \ + .eint_type = EINT_TYPE_GPIO, \ + .eint_offset = offs, \ + .eint_fltcon_offset = fltcon_offs, \ + .name = id \ + } + +#define GS101_PIN_BANK_EINTW(pins, reg, id, offs, fltcon_offs) \ + { \ + .type = &exynos850_bank_type_alive, \ + .pctl_offset = reg, \ + .nr_pins = pins, \ + .eint_type = EINT_TYPE_WKUP, \ + .eint_offset = offs, \ + .eint_fltcon_offset = fltcon_offs, \ + .name = id \ + } + /** * struct exynos_weint_data: irq specific data for all the wakeup interrupts * generated by the external wakeup interrupt controller. diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c index cfced7afd4ca..963060920301 100644 --- a/drivers/pinctrl/samsung/pinctrl-samsung.c +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c @@ -1230,6 +1230,7 @@ samsung_pinctrl_get_soc_data(struct samsung_pinctrl_drv_data *d, bank->eint_con_offset = bdata->eint_con_offset; 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->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 bb0689d52ea0..371e4f02bbfb 100644 --- a/drivers/pinctrl/samsung/pinctrl-samsung.h +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h @@ -144,6 +144,7 @@ struct samsung_pin_bank_type { * @eint_con_offset: ExynosAuto SoC-specific EINT control register offset of bank. * @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. * @name: name to be prefixed for each pin in this pin bank. */ struct samsung_pin_bank_data { @@ -158,6 +159,7 @@ struct samsung_pin_bank_data { u32 eint_con_offset; u32 eint_mask_offset; u32 eint_pend_offset; + u32 eint_fltcon_offset; const char *name; }; @@ -175,6 +177,7 @@ struct samsung_pin_bank_data { * @eint_con_offset: ExynosAuto SoC-specific EINT register or interrupt offset of bank. * @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. * @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. @@ -201,6 +204,7 @@ struct samsung_pin_bank { u32 eint_con_offset; u32 eint_mask_offset; u32 eint_pend_offset; + u32 eint_fltcon_offset; const char *name; u32 id; -- 2.48.1.711.g2feabab25a-goog ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/4] pinctrl: samsung: add support for eint_fltcon_offset 2025-03-01 11:43 ` [PATCH v2 1/4] pinctrl: samsung: add support for eint_fltcon_offset Peter Griffin @ 2025-03-06 11:24 ` André Draszik 0 siblings, 0 replies; 11+ messages in thread From: André Draszik @ 2025-03-06 11:24 UTC (permalink / raw) To: Peter Griffin, Krzysztof Kozlowski, Sylwester Nawrocki, Alim Akhtar, Linus Walleij Cc: linux-arm-kernel, linux-samsung-soc, linux-gpio, linux-kernel, tudor.ambarus, willmcvicker, semen.protsenko, kernel-team, jaewon02.kim, stable On Sat, 2025-03-01 at 11:43 +0000, Peter Griffin wrote: > On gs101 SoC the fltcon0 (filter configuration 0) offset > isn't at a fixed offset like previous SoCs as the fltcon1 > register only exists when there are more than 4 pins in the > bank. > > Add a eint_fltcon_offset and new GS101_PIN_BANK_EINT* > macros that take an additional fltcon_offs variable. > > This can then be used in suspend/resume callbacks to > save and restore the fltcon0 and fltcon1 registers. > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > Fixes: 4a8be01a1a7a ("pinctrl: samsung: Add gs101 SoC pinctrl configuration") > Cc: stable@vger.kernel.org Reviewed-by: André Draszik <andre.draszik@linaro.org> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/4] pinctrl: samsung: add dedicated SoC eint suspend/resume callbacks 2025-03-01 11:43 [PATCH v2 0/4] samsung: pinctrl: Add support for eint_fltcon_offset and filter selection on gs101 Peter Griffin 2025-03-01 11:43 ` [PATCH v2 1/4] pinctrl: samsung: add support for eint_fltcon_offset Peter Griffin @ 2025-03-01 11:43 ` Peter Griffin 2025-03-06 11:57 ` André Draszik 2025-03-01 11:43 ` [PATCH v2 3/4] pinctrl: samsung: add gs101 specific " Peter Griffin 2025-03-01 11:43 ` [PATCH v2 4/4] pinctrl: samsung: Add filter selection support for alive bank on gs101 Peter Griffin 3 siblings, 1 reply; 11+ messages in thread From: Peter Griffin @ 2025-03-01 11:43 UTC (permalink / raw) To: Krzysztof Kozlowski, Sylwester Nawrocki, Alim Akhtar, Linus Walleij Cc: linux-arm-kernel, linux-samsung-soc, linux-gpio, linux-kernel, andre.draszik, tudor.ambarus, willmcvicker, semen.protsenko, kernel-team, jaewon02.kim, Peter Griffin gs101 needs it's own suspend/resume callbacks to use the newly added eint_fltcon_offset for saving & restoring fltcon0 & fltcon1 registers. It also differs to previous SoCs in that fltcon1 register doesn't always exist for each bank. exynosautov920 also has dedicated logic for using eint_con_offset and eint_mask_offset for saving & restoring it's registers. Refactor the existing platform specific suspend/resume callback so that each SoC variant has their own callback containing the SoC specific logic. Additionally we now call drvdata->suspend() & drvdata->resume() from within the loop that iterates the banks in samsung_pinctrl_suspend() and samsung_pinctrl_resume(). This simplifies the logic, and allows us to remove the clk_enable() and clk_disable() from the callbacks. Signed-off-by: Peter Griffin <peter.griffin@linaro.org> --- Changes since v1: * Split code refactor & gs101 parts into separate patches (Andre) --- drivers/pinctrl/samsung/pinctrl-exynos-arm64.c | 28 ++-- drivers/pinctrl/samsung/pinctrl-exynos.c | 201 ++++++++++--------------- drivers/pinctrl/samsung/pinctrl-exynos.h | 6 +- drivers/pinctrl/samsung/pinctrl-samsung.c | 11 +- drivers/pinctrl/samsung/pinctrl-samsung.h | 8 +- 5 files changed, 111 insertions(+), 143 deletions(-) diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c index e28fe8177646..57c98d2451b5 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c @@ -1112,8 +1112,8 @@ static const struct samsung_pin_ctrl exynosautov920_pin_ctrl[] = { .pin_banks = exynosautov920_pin_banks0, .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks0), .eint_wkup_init = exynos_eint_wkup_init, - .suspend = exynos_pinctrl_suspend, - .resume = exynos_pinctrl_resume, + .suspend = exynosautov920_pinctrl_suspend, + .resume = exynosautov920_pinctrl_resume, .retention_data = &exynosautov920_retention_data, }, { /* pin-controller instance 1 AUD data */ @@ -1124,43 +1124,43 @@ static const struct samsung_pin_ctrl exynosautov920_pin_ctrl[] = { .pin_banks = exynosautov920_pin_banks2, .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks2), .eint_gpio_init = exynos_eint_gpio_init, - .suspend = exynos_pinctrl_suspend, - .resume = exynos_pinctrl_resume, + .suspend = exynosautov920_pinctrl_suspend, + .resume = exynosautov920_pinctrl_resume, }, { /* pin-controller instance 3 HSI1 data */ .pin_banks = exynosautov920_pin_banks3, .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks3), .eint_gpio_init = exynos_eint_gpio_init, - .suspend = exynos_pinctrl_suspend, - .resume = exynos_pinctrl_resume, + .suspend = exynosautov920_pinctrl_suspend, + .resume = exynosautov920_pinctrl_resume, }, { /* pin-controller instance 4 HSI2 data */ .pin_banks = exynosautov920_pin_banks4, .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks4), .eint_gpio_init = exynos_eint_gpio_init, - .suspend = exynos_pinctrl_suspend, - .resume = exynos_pinctrl_resume, + .suspend = exynosautov920_pinctrl_suspend, + .resume = exynosautov920_pinctrl_resume, }, { /* pin-controller instance 5 HSI2UFS data */ .pin_banks = exynosautov920_pin_banks5, .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks5), .eint_gpio_init = exynos_eint_gpio_init, - .suspend = exynos_pinctrl_suspend, - .resume = exynos_pinctrl_resume, + .suspend = exynosautov920_pinctrl_suspend, + .resume = exynosautov920_pinctrl_resume, }, { /* pin-controller instance 6 PERIC0 data */ .pin_banks = exynosautov920_pin_banks6, .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks6), .eint_gpio_init = exynos_eint_gpio_init, - .suspend = exynos_pinctrl_suspend, - .resume = exynos_pinctrl_resume, + .suspend = exynosautov920_pinctrl_suspend, + .resume = exynosautov920_pinctrl_resume, }, { /* pin-controller instance 7 PERIC1 data */ .pin_banks = exynosautov920_pin_banks7, .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks7), .eint_gpio_init = exynos_eint_gpio_init, - .suspend = exynos_pinctrl_suspend, - .resume = exynos_pinctrl_resume, + .suspend = exynosautov920_pinctrl_suspend, + .resume = exynosautov920_pinctrl_resume, }, }; diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c index ac6dc22b37c9..d65a9fba0781 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c @@ -761,153 +761,118 @@ __init int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) return 0; } -static void exynos_pinctrl_suspend_bank( - struct samsung_pinctrl_drv_data *drvdata, - struct samsung_pin_bank *bank) +static void exynos_set_wakeup(struct samsung_pin_bank *bank) { - struct exynos_eint_gpio_save *save = bank->soc_priv; - const void __iomem *regs = bank->eint_base; + struct exynos_irq_chip *irq_chip = NULL; - if (clk_enable(bank->drvdata->pclk)) { - dev_err(bank->gpio_chip.parent, - "unable to enable clock for saving state\n"); - return; + if (bank->eint_type == EINT_TYPE_WKUP) { + if (bank->irq_chip) { + irq_chip = bank->irq_chip; + irq_chip->set_eint_wakeup_mask(bank->drvdata, irq_chip); + } } - - save->eint_con = readl(regs + EXYNOS_GPIO_ECON_OFFSET - + bank->eint_offset); - save->eint_fltcon0 = readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET - + 2 * bank->eint_offset); - save->eint_fltcon1 = readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET - + 2 * bank->eint_offset + 4); - save->eint_mask = readl(regs + bank->irq_chip->eint_mask - + bank->eint_offset); - - clk_disable(bank->drvdata->pclk); - - pr_debug("%s: save con %#010x\n", bank->name, save->eint_con); - pr_debug("%s: save fltcon0 %#010x\n", bank->name, save->eint_fltcon0); - pr_debug("%s: save fltcon1 %#010x\n", bank->name, save->eint_fltcon1); - pr_debug("%s: save mask %#010x\n", bank->name, save->eint_mask); } -static void exynosauto_pinctrl_suspend_bank(struct samsung_pinctrl_drv_data *drvdata, - struct samsung_pin_bank *bank) +void exynos_pinctrl_suspend(struct samsung_pin_bank *bank) { struct exynos_eint_gpio_save *save = bank->soc_priv; const void __iomem *regs = bank->eint_base; - if (clk_enable(bank->drvdata->pclk)) { - dev_err(bank->gpio_chip.parent, - "unable to enable clock for saving state\n"); - return; + exynos_set_wakeup(bank); + + if (bank->eint_type == EINT_TYPE_GPIO) { + save->eint_con = readl(regs + EXYNOS_GPIO_ECON_OFFSET + + bank->eint_offset); + save->eint_fltcon0 = readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET + + 2 * bank->eint_offset); + save->eint_fltcon1 = readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET + + 2 * bank->eint_offset + 4); + save->eint_mask = readl(regs + bank->irq_chip->eint_mask + + bank->eint_offset); + + pr_debug("%s: save con %#010x\n", + bank->name, save->eint_con); + pr_debug("%s: save fltcon0 %#010x\n", + bank->name, save->eint_fltcon0); + pr_debug("%s: save fltcon1 %#010x\n", + bank->name, save->eint_fltcon1); + pr_debug("%s: save mask %#010x\n", + bank->name, save->eint_mask); } - - save->eint_con = readl(regs + bank->pctl_offset + bank->eint_con_offset); - save->eint_mask = readl(regs + bank->pctl_offset + bank->eint_mask_offset); - - clk_disable(bank->drvdata->pclk); - - pr_debug("%s: save con %#010x\n", bank->name, save->eint_con); - pr_debug("%s: save mask %#010x\n", bank->name, save->eint_mask); } -void exynos_pinctrl_suspend(struct samsung_pinctrl_drv_data *drvdata) +void exynosautov920_pinctrl_suspend(struct samsung_pin_bank *bank) { - struct samsung_pin_bank *bank = drvdata->pin_banks; - struct exynos_irq_chip *irq_chip = NULL; - int i; + struct exynos_eint_gpio_save *save = bank->soc_priv; + const void __iomem *regs = bank->eint_base; - for (i = 0; i < drvdata->nr_banks; ++i, ++bank) { - if (bank->eint_type == EINT_TYPE_GPIO) { - if (bank->eint_con_offset) - exynosauto_pinctrl_suspend_bank(drvdata, bank); - else - exynos_pinctrl_suspend_bank(drvdata, bank); - } - else if (bank->eint_type == EINT_TYPE_WKUP) { - if (!irq_chip) { - irq_chip = bank->irq_chip; - irq_chip->set_eint_wakeup_mask(drvdata, - irq_chip); - } - } + exynos_set_wakeup(bank); + + if (bank->eint_type == EINT_TYPE_GPIO) { + save->eint_con = readl(regs + bank->pctl_offset + + bank->eint_con_offset); + save->eint_mask = readl(regs + bank->pctl_offset + + bank->eint_mask_offset); + pr_debug("%s: save con %#010x\n", + bank->name, save->eint_con); + pr_debug("%s: save mask %#010x\n", + bank->name, save->eint_mask); } } -static void exynos_pinctrl_resume_bank( - struct samsung_pinctrl_drv_data *drvdata, - struct samsung_pin_bank *bank) +void exynos_pinctrl_resume(struct samsung_pin_bank *bank) { struct exynos_eint_gpio_save *save = bank->soc_priv; void __iomem *regs = bank->eint_base; - if (clk_enable(bank->drvdata->pclk)) { - dev_err(bank->gpio_chip.parent, - "unable to enable clock for restoring state\n"); - return; + if (bank->eint_type == EINT_TYPE_GPIO) { + pr_debug("%s: con %#010x => %#010x\n", bank->name, + readl(regs + EXYNOS_GPIO_ECON_OFFSET + + bank->eint_offset), save->eint_con); + pr_debug("%s: fltcon0 %#010x => %#010x\n", bank->name, + readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET + + 2 * bank->eint_offset), save->eint_fltcon0); + pr_debug("%s: fltcon1 %#010x => %#010x\n", bank->name, + readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET + + 2 * bank->eint_offset + 4), + save->eint_fltcon1); + pr_debug("%s: mask %#010x => %#010x\n", bank->name, + readl(regs + bank->irq_chip->eint_mask + + bank->eint_offset), save->eint_mask); + + writel(save->eint_con, regs + EXYNOS_GPIO_ECON_OFFSET + + bank->eint_offset); + writel(save->eint_fltcon0, regs + EXYNOS_GPIO_EFLTCON_OFFSET + + 2 * bank->eint_offset); + writel(save->eint_fltcon1, regs + EXYNOS_GPIO_EFLTCON_OFFSET + + 2 * bank->eint_offset + 4); + writel(save->eint_mask, regs + bank->irq_chip->eint_mask + + bank->eint_offset); } - - pr_debug("%s: con %#010x => %#010x\n", bank->name, - readl(regs + EXYNOS_GPIO_ECON_OFFSET - + bank->eint_offset), save->eint_con); - pr_debug("%s: fltcon0 %#010x => %#010x\n", bank->name, - readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET - + 2 * bank->eint_offset), save->eint_fltcon0); - pr_debug("%s: fltcon1 %#010x => %#010x\n", bank->name, - readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET - + 2 * bank->eint_offset + 4), save->eint_fltcon1); - pr_debug("%s: mask %#010x => %#010x\n", bank->name, - readl(regs + bank->irq_chip->eint_mask - + bank->eint_offset), save->eint_mask); - - writel(save->eint_con, regs + EXYNOS_GPIO_ECON_OFFSET - + bank->eint_offset); - writel(save->eint_fltcon0, regs + EXYNOS_GPIO_EFLTCON_OFFSET - + 2 * bank->eint_offset); - writel(save->eint_fltcon1, regs + EXYNOS_GPIO_EFLTCON_OFFSET - + 2 * bank->eint_offset + 4); - writel(save->eint_mask, regs + bank->irq_chip->eint_mask - + bank->eint_offset); - - clk_disable(bank->drvdata->pclk); } -static void exynosauto_pinctrl_resume_bank(struct samsung_pinctrl_drv_data *drvdata, - struct samsung_pin_bank *bank) +void exynosautov920_pinctrl_resume(struct samsung_pin_bank *bank) { struct exynos_eint_gpio_save *save = bank->soc_priv; void __iomem *regs = bank->eint_base; - if (clk_enable(bank->drvdata->pclk)) { - dev_err(bank->gpio_chip.parent, - "unable to enable clock for restoring state\n"); - return; + if (bank->eint_type == EINT_TYPE_GPIO) { + /* exynosautov920 has eint_con_offset for all but one bank */ + if (!bank->eint_con_offset) + exynos_pinctrl_resume(bank); + + pr_debug("%s: con %#010x => %#010x\n", bank->name, + readl(regs + bank->pctl_offset + bank->eint_con_offset), + save->eint_con); + pr_debug("%s: mask %#010x => %#010x\n", bank->name, + readl(regs + bank->pctl_offset + + bank->eint_mask_offset), save->eint_mask); + + writel(save->eint_con, + regs + bank->pctl_offset + bank->eint_con_offset); + writel(save->eint_mask, + regs + bank->pctl_offset + bank->eint_mask_offset); } - - pr_debug("%s: con %#010x => %#010x\n", bank->name, - readl(regs + bank->pctl_offset + bank->eint_con_offset), save->eint_con); - pr_debug("%s: mask %#010x => %#010x\n", bank->name, - readl(regs + bank->pctl_offset + bank->eint_mask_offset), save->eint_mask); - - writel(save->eint_con, regs + bank->pctl_offset + bank->eint_con_offset); - writel(save->eint_mask, regs + bank->pctl_offset + bank->eint_mask_offset); - - clk_disable(bank->drvdata->pclk); -} - -void exynos_pinctrl_resume(struct samsung_pinctrl_drv_data *drvdata) -{ - struct samsung_pin_bank *bank = drvdata->pin_banks; - int i; - - for (i = 0; i < drvdata->nr_banks; ++i, ++bank) - if (bank->eint_type == EINT_TYPE_GPIO) { - if (bank->eint_con_offset) - exynosauto_pinctrl_resume_bank(drvdata, bank); - else - exynos_pinctrl_resume_bank(drvdata, bank); - } } 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 33df21d5c9d6..35c2bc4ea488 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.h +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h @@ -221,8 +221,10 @@ struct exynos_muxed_weint_data { int exynos_eint_gpio_init(struct samsung_pinctrl_drv_data *d); int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d); -void exynos_pinctrl_suspend(struct samsung_pinctrl_drv_data *drvdata); -void exynos_pinctrl_resume(struct samsung_pinctrl_drv_data *drvdata); +void exynosautov920_pinctrl_resume(struct samsung_pin_bank *bank); +void exynosautov920_pinctrl_suspend(struct samsung_pin_bank *bank); +void exynos_pinctrl_suspend(struct samsung_pin_bank *bank); +void exynos_pinctrl_resume(struct samsung_pin_bank *bank); struct samsung_retention_ctrl * exynos_retention_init(struct samsung_pinctrl_drv_data *drvdata, const struct samsung_retention_data *data); diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c index 963060920301..375634d8cc79 100644 --- a/drivers/pinctrl/samsung/pinctrl-samsung.c +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c @@ -1349,6 +1349,9 @@ static int __maybe_unused samsung_pinctrl_suspend(struct device *dev) const u8 *widths = bank->type->fld_width; enum pincfg_type type; + if (drvdata->suspend) + drvdata->suspend(bank); + /* Registers without a powerdown config aren't lost */ if (!widths[PINCFG_TYPE_CON_PDN]) continue; @@ -1373,8 +1376,6 @@ static int __maybe_unused samsung_pinctrl_suspend(struct device *dev) clk_disable(drvdata->pclk); - if (drvdata->suspend) - drvdata->suspend(drvdata); if (drvdata->retention_ctrl && drvdata->retention_ctrl->enable) drvdata->retention_ctrl->enable(drvdata); @@ -1406,9 +1407,6 @@ static int __maybe_unused samsung_pinctrl_resume(struct device *dev) return ret; } - if (drvdata->resume) - drvdata->resume(drvdata); - for (i = 0; i < drvdata->nr_banks; i++) { struct samsung_pin_bank *bank = &drvdata->pin_banks[i]; void __iomem *reg = bank->pctl_base + bank->pctl_offset; @@ -1416,6 +1414,9 @@ static int __maybe_unused samsung_pinctrl_resume(struct device *dev) const u8 *widths = bank->type->fld_width; enum pincfg_type type; + if (drvdata->resume) + drvdata->resume(bank); + /* Registers without a powerdown config aren't lost */ if (!widths[PINCFG_TYPE_CON_PDN]) continue; diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h b/drivers/pinctrl/samsung/pinctrl-samsung.h index 371e4f02bbfb..e939e5bb0347 100644 --- a/drivers/pinctrl/samsung/pinctrl-samsung.h +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h @@ -285,8 +285,8 @@ struct samsung_pin_ctrl { int (*eint_gpio_init)(struct samsung_pinctrl_drv_data *); int (*eint_wkup_init)(struct samsung_pinctrl_drv_data *); void (*pud_value_init)(struct samsung_pinctrl_drv_data *drvdata); - void (*suspend)(struct samsung_pinctrl_drv_data *); - void (*resume)(struct samsung_pinctrl_drv_data *); + void (*suspend)(struct samsung_pin_bank *bank); + void (*resume)(struct samsung_pin_bank *bank); }; /** @@ -335,8 +335,8 @@ struct samsung_pinctrl_drv_data { struct samsung_retention_ctrl *retention_ctrl; - void (*suspend)(struct samsung_pinctrl_drv_data *); - void (*resume)(struct samsung_pinctrl_drv_data *); + void (*suspend)(struct samsung_pin_bank *bank); + void (*resume)(struct samsung_pin_bank *bank); }; /** -- 2.48.1.711.g2feabab25a-goog ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/4] pinctrl: samsung: add dedicated SoC eint suspend/resume callbacks 2025-03-01 11:43 ` [PATCH v2 2/4] pinctrl: samsung: add dedicated SoC eint suspend/resume callbacks Peter Griffin @ 2025-03-06 11:57 ` André Draszik 2025-03-06 15:12 ` André Draszik 2025-03-06 20:47 ` Peter Griffin 0 siblings, 2 replies; 11+ messages in thread From: André Draszik @ 2025-03-06 11:57 UTC (permalink / raw) To: Peter Griffin, Krzysztof Kozlowski, Sylwester Nawrocki, Alim Akhtar, Linus Walleij Cc: linux-arm-kernel, linux-samsung-soc, linux-gpio, linux-kernel, tudor.ambarus, willmcvicker, semen.protsenko, kernel-team, jaewon02.kim On Sat, 2025-03-01 at 11:43 +0000, Peter Griffin wrote: > gs101 needs it's own suspend/resume callbacks to use the newly > added eint_fltcon_offset for saving & restoring fltcon0 & fltcon1 > registers. It also differs to previous SoCs in that fltcon1 > register doesn't always exist for each bank. > > exynosautov920 also has dedicated logic for using eint_con_offset > and eint_mask_offset for saving & restoring it's registers. > > Refactor the existing platform specific suspend/resume callback > so that each SoC variant has their own callback containing the > SoC specific logic. > > Additionally we now call drvdata->suspend() & drvdata->resume() > from within the loop that iterates the banks in > samsung_pinctrl_suspend() and samsung_pinctrl_resume(). > > This simplifies the logic, and allows us to remove the > clk_enable() and clk_disable() from the callbacks. > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > --- > Changes since v1: > * Split code refactor & gs101 parts into separate patches (Andre) > --- > drivers/pinctrl/samsung/pinctrl-exynos-arm64.c | 28 ++-- > drivers/pinctrl/samsung/pinctrl-exynos.c | 201 ++++++++++--------------- > drivers/pinctrl/samsung/pinctrl-exynos.h | 6 +- > drivers/pinctrl/samsung/pinctrl-samsung.c | 11 +- > drivers/pinctrl/samsung/pinctrl-samsung.h | 8 +- > 5 files changed, 111 insertions(+), 143 deletions(-) > > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c > index e28fe8177646..57c98d2451b5 100644 > --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c > +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c > @@ -1112,8 +1112,8 @@ static const struct samsung_pin_ctrl exynosautov920_pin_ctrl[] = { > .pin_banks = exynosautov920_pin_banks0, > .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks0), > .eint_wkup_init = exynos_eint_wkup_init, > - .suspend = exynos_pinctrl_suspend, > - .resume = exynos_pinctrl_resume, > + .suspend = exynosautov920_pinctrl_suspend, > + .resume = exynosautov920_pinctrl_resume, > .retention_data = &exynosautov920_retention_data, > }, { > /* pin-controller instance 1 AUD data */ > @@ -1124,43 +1124,43 @@ static const struct samsung_pin_ctrl exynosautov920_pin_ctrl[] = { > .pin_banks = exynosautov920_pin_banks2, > .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks2), > .eint_gpio_init = exynos_eint_gpio_init, > - .suspend = exynos_pinctrl_suspend, > - .resume = exynos_pinctrl_resume, > + .suspend = exynosautov920_pinctrl_suspend, > + .resume = exynosautov920_pinctrl_resume, > }, { > /* pin-controller instance 3 HSI1 data */ > .pin_banks = exynosautov920_pin_banks3, > .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks3), > .eint_gpio_init = exynos_eint_gpio_init, > - .suspend = exynos_pinctrl_suspend, > - .resume = exynos_pinctrl_resume, > + .suspend = exynosautov920_pinctrl_suspend, > + .resume = exynosautov920_pinctrl_resume, > }, { > /* pin-controller instance 4 HSI2 data */ > .pin_banks = exynosautov920_pin_banks4, > .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks4), > .eint_gpio_init = exynos_eint_gpio_init, > - .suspend = exynos_pinctrl_suspend, > - .resume = exynos_pinctrl_resume, > + .suspend = exynosautov920_pinctrl_suspend, > + .resume = exynosautov920_pinctrl_resume, > }, { > /* pin-controller instance 5 HSI2UFS data */ > .pin_banks = exynosautov920_pin_banks5, > .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks5), > .eint_gpio_init = exynos_eint_gpio_init, > - .suspend = exynos_pinctrl_suspend, > - .resume = exynos_pinctrl_resume, > + .suspend = exynosautov920_pinctrl_suspend, > + .resume = exynosautov920_pinctrl_resume, > }, { > /* pin-controller instance 6 PERIC0 data */ > .pin_banks = exynosautov920_pin_banks6, > .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks6), > .eint_gpio_init = exynos_eint_gpio_init, > - .suspend = exynos_pinctrl_suspend, > - .resume = exynos_pinctrl_resume, > + .suspend = exynosautov920_pinctrl_suspend, > + .resume = exynosautov920_pinctrl_resume, > }, { > /* pin-controller instance 7 PERIC1 data */ > .pin_banks = exynosautov920_pin_banks7, > .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks7), > .eint_gpio_init = exynos_eint_gpio_init, > - .suspend = exynos_pinctrl_suspend, > - .resume = exynos_pinctrl_resume, > + .suspend = exynosautov920_pinctrl_suspend, > + .resume = exynosautov920_pinctrl_resume, > }, > }; > > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c > index ac6dc22b37c9..d65a9fba0781 100644 > --- a/drivers/pinctrl/samsung/pinctrl-exynos.c > +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c > @@ -761,153 +761,118 @@ __init int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) > return 0; > } > > -static void exynos_pinctrl_suspend_bank( > - struct samsung_pinctrl_drv_data *drvdata, > - struct samsung_pin_bank *bank) > +static void exynos_set_wakeup(struct samsung_pin_bank *bank) > { > - struct exynos_eint_gpio_save *save = bank->soc_priv; > - const void __iomem *regs = bank->eint_base; > + struct exynos_irq_chip *irq_chip = NULL; This is a useless init and the value is overwritten right away. > > - if (clk_enable(bank->drvdata->pclk)) { > - dev_err(bank->gpio_chip.parent, > - "unable to enable clock for saving state\n"); > - return; > + if (bank->eint_type == EINT_TYPE_WKUP) { > + if (bank->irq_chip) { > + irq_chip = bank->irq_chip; > + irq_chip->set_eint_wakeup_mask(bank->drvdata, irq_chip); > + } > } > - > - save->eint_con = readl(regs + EXYNOS_GPIO_ECON_OFFSET > - + bank->eint_offset); > - save->eint_fltcon0 = readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > - + 2 * bank->eint_offset); > - save->eint_fltcon1 = readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > - + 2 * bank->eint_offset + 4); > - save->eint_mask = readl(regs + bank->irq_chip->eint_mask > - + bank->eint_offset); > - > - clk_disable(bank->drvdata->pclk); > - > - pr_debug("%s: save con %#010x\n", bank->name, save->eint_con); > - pr_debug("%s: save fltcon0 %#010x\n", bank->name, save->eint_fltcon0); > - pr_debug("%s: save fltcon1 %#010x\n", bank->name, save->eint_fltcon1); > - pr_debug("%s: save mask %#010x\n", bank->name, save->eint_mask); > } > > -static void exynosauto_pinctrl_suspend_bank(struct samsung_pinctrl_drv_data *drvdata, > - struct samsung_pin_bank *bank) > +void exynos_pinctrl_suspend(struct samsung_pin_bank *bank) > { > struct exynos_eint_gpio_save *save = bank->soc_priv; > const void __iomem *regs = bank->eint_base; > > - if (clk_enable(bank->drvdata->pclk)) { > - dev_err(bank->gpio_chip.parent, > - "unable to enable clock for saving state\n"); > - return; > + exynos_set_wakeup(bank); > + > + if (bank->eint_type == EINT_TYPE_GPIO) { Can you swap the order, and make the call to exynos_set_wakeup() an else, to avoid some needless tests and make it clearer that set_wakeup() is conditional as well, i.e.: if (bank->eint_type == EINT_TYPE_GPIO) { ... } else { exynos_set_wakeup(bank); } What makes it hard to read is that as-is, it looks like the set_wakeup() is unconditional, while it's also based on bank->eint_type (which is also more obvious before your patch). Or maybe even have exynos_set_wakeup() return a bool, as in the follow-up gs101 patch, there are three tests of this type. Other than that, I think I looks good. Cheers, Andre' > + save->eint_con = readl(regs + EXYNOS_GPIO_ECON_OFFSET > + + bank->eint_offset); > + save->eint_fltcon0 = readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > + + 2 * bank->eint_offset); > + save->eint_fltcon1 = readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > + + 2 * bank->eint_offset + 4); > + save->eint_mask = readl(regs + bank->irq_chip->eint_mask > + + bank->eint_offset); > + > + pr_debug("%s: save con %#010x\n", > + bank->name, save->eint_con); > + pr_debug("%s: save fltcon0 %#010x\n", > + bank->name, save->eint_fltcon0); > + pr_debug("%s: save fltcon1 %#010x\n", > + bank->name, save->eint_fltcon1); > + pr_debug("%s: save mask %#010x\n", > + bank->name, save->eint_mask); > } > - > - save->eint_con = readl(regs + bank->pctl_offset + bank->eint_con_offset); > - save->eint_mask = readl(regs + bank->pctl_offset + bank->eint_mask_offset); > - > - clk_disable(bank->drvdata->pclk); > - > - pr_debug("%s: save con %#010x\n", bank->name, save->eint_con); > - pr_debug("%s: save mask %#010x\n", bank->name, save->eint_mask); > } > > -void exynos_pinctrl_suspend(struct samsung_pinctrl_drv_data *drvdata) > +void exynosautov920_pinctrl_suspend(struct samsung_pin_bank *bank) > { > - struct samsung_pin_bank *bank = drvdata->pin_banks; > - struct exynos_irq_chip *irq_chip = NULL; > - int i; > + struct exynos_eint_gpio_save *save = bank->soc_priv; > + const void __iomem *regs = bank->eint_base; > > - for (i = 0; i < drvdata->nr_banks; ++i, ++bank) { > - if (bank->eint_type == EINT_TYPE_GPIO) { > - if (bank->eint_con_offset) > - exynosauto_pinctrl_suspend_bank(drvdata, bank); > - else > - exynos_pinctrl_suspend_bank(drvdata, bank); > - } > - else if (bank->eint_type == EINT_TYPE_WKUP) { > - if (!irq_chip) { > - irq_chip = bank->irq_chip; > - irq_chip->set_eint_wakeup_mask(drvdata, > - irq_chip); > - } > - } > + exynos_set_wakeup(bank); > + > + if (bank->eint_type == EINT_TYPE_GPIO) { > + save->eint_con = readl(regs + bank->pctl_offset + > + bank->eint_con_offset); > + save->eint_mask = readl(regs + bank->pctl_offset + > + bank->eint_mask_offset); > + pr_debug("%s: save con %#010x\n", > + bank->name, save->eint_con); > + pr_debug("%s: save mask %#010x\n", > + bank->name, save->eint_mask); > } > } > > -static void exynos_pinctrl_resume_bank( > - struct samsung_pinctrl_drv_data *drvdata, > - struct samsung_pin_bank *bank) > +void exynos_pinctrl_resume(struct samsung_pin_bank *bank) > { > struct exynos_eint_gpio_save *save = bank->soc_priv; > void __iomem *regs = bank->eint_base; > > - if (clk_enable(bank->drvdata->pclk)) { > - dev_err(bank->gpio_chip.parent, > - "unable to enable clock for restoring state\n"); > - return; > + if (bank->eint_type == EINT_TYPE_GPIO) { > + pr_debug("%s: con %#010x => %#010x\n", bank->name, > + readl(regs + EXYNOS_GPIO_ECON_OFFSET > + + bank->eint_offset), save->eint_con); > + pr_debug("%s: fltcon0 %#010x => %#010x\n", bank->name, > + readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > + + 2 * bank->eint_offset), save->eint_fltcon0); > + pr_debug("%s: fltcon1 %#010x => %#010x\n", bank->name, > + readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > + + 2 * bank->eint_offset + 4), > + save->eint_fltcon1); > + pr_debug("%s: mask %#010x => %#010x\n", bank->name, > + readl(regs + bank->irq_chip->eint_mask > + + bank->eint_offset), save->eint_mask); > + > + writel(save->eint_con, regs + EXYNOS_GPIO_ECON_OFFSET > + + bank->eint_offset); > + writel(save->eint_fltcon0, regs + EXYNOS_GPIO_EFLTCON_OFFSET > + + 2 * bank->eint_offset); > + writel(save->eint_fltcon1, regs + EXYNOS_GPIO_EFLTCON_OFFSET > + + 2 * bank->eint_offset + 4); > + writel(save->eint_mask, regs + bank->irq_chip->eint_mask > + + bank->eint_offset); > } > - > - pr_debug("%s: con %#010x => %#010x\n", bank->name, > - readl(regs + EXYNOS_GPIO_ECON_OFFSET > - + bank->eint_offset), save->eint_con); > - pr_debug("%s: fltcon0 %#010x => %#010x\n", bank->name, > - readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > - + 2 * bank->eint_offset), save->eint_fltcon0); > - pr_debug("%s: fltcon1 %#010x => %#010x\n", bank->name, > - readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > - + 2 * bank->eint_offset + 4), save->eint_fltcon1); > - pr_debug("%s: mask %#010x => %#010x\n", bank->name, > - readl(regs + bank->irq_chip->eint_mask > - + bank->eint_offset), save->eint_mask); > - > - writel(save->eint_con, regs + EXYNOS_GPIO_ECON_OFFSET > - + bank->eint_offset); > - writel(save->eint_fltcon0, regs + EXYNOS_GPIO_EFLTCON_OFFSET > - + 2 * bank->eint_offset); > - writel(save->eint_fltcon1, regs + EXYNOS_GPIO_EFLTCON_OFFSET > - + 2 * bank->eint_offset + 4); > - writel(save->eint_mask, regs + bank->irq_chip->eint_mask > - + bank->eint_offset); > - > - clk_disable(bank->drvdata->pclk); > } > > -static void exynosauto_pinctrl_resume_bank(struct samsung_pinctrl_drv_data *drvdata, > - struct samsung_pin_bank *bank) > +void exynosautov920_pinctrl_resume(struct samsung_pin_bank *bank) > { > struct exynos_eint_gpio_save *save = bank->soc_priv; > void __iomem *regs = bank->eint_base; > > - if (clk_enable(bank->drvdata->pclk)) { > - dev_err(bank->gpio_chip.parent, > - "unable to enable clock for restoring state\n"); > - return; > + if (bank->eint_type == EINT_TYPE_GPIO) { > + /* exynosautov920 has eint_con_offset for all but one bank */ > + if (!bank->eint_con_offset) > + exynos_pinctrl_resume(bank); > + > + pr_debug("%s: con %#010x => %#010x\n", bank->name, > + readl(regs + bank->pctl_offset + bank->eint_con_offset), > + save->eint_con); > + pr_debug("%s: mask %#010x => %#010x\n", bank->name, > + readl(regs + bank->pctl_offset + > + bank->eint_mask_offset), save->eint_mask); > + > + writel(save->eint_con, > + regs + bank->pctl_offset + bank->eint_con_offset); > + writel(save->eint_mask, > + regs + bank->pctl_offset + bank->eint_mask_offset); > } > - > - pr_debug("%s: con %#010x => %#010x\n", bank->name, > - readl(regs + bank->pctl_offset + bank->eint_con_offset), save->eint_con); > - pr_debug("%s: mask %#010x => %#010x\n", bank->name, > - readl(regs + bank->pctl_offset + bank->eint_mask_offset), save->eint_mask); > - > - writel(save->eint_con, regs + bank->pctl_offset + bank->eint_con_offset); > - writel(save->eint_mask, regs + bank->pctl_offset + bank->eint_mask_offset); > - > - clk_disable(bank->drvdata->pclk); > -} > - > -void exynos_pinctrl_resume(struct samsung_pinctrl_drv_data *drvdata) > -{ > - struct samsung_pin_bank *bank = drvdata->pin_banks; > - int i; > - > - for (i = 0; i < drvdata->nr_banks; ++i, ++bank) > - if (bank->eint_type == EINT_TYPE_GPIO) { > - if (bank->eint_con_offset) > - exynosauto_pinctrl_resume_bank(drvdata, bank); > - else > - exynos_pinctrl_resume_bank(drvdata, bank); > - } > } > > 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 33df21d5c9d6..35c2bc4ea488 100644 > --- a/drivers/pinctrl/samsung/pinctrl-exynos.h > +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h > @@ -221,8 +221,10 @@ struct exynos_muxed_weint_data { > > int exynos_eint_gpio_init(struct samsung_pinctrl_drv_data *d); > int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d); > -void exynos_pinctrl_suspend(struct samsung_pinctrl_drv_data *drvdata); > -void exynos_pinctrl_resume(struct samsung_pinctrl_drv_data *drvdata); > +void exynosautov920_pinctrl_resume(struct samsung_pin_bank *bank); > +void exynosautov920_pinctrl_suspend(struct samsung_pin_bank *bank); > +void exynos_pinctrl_suspend(struct samsung_pin_bank *bank); > +void exynos_pinctrl_resume(struct samsung_pin_bank *bank); > struct samsung_retention_ctrl * > exynos_retention_init(struct samsung_pinctrl_drv_data *drvdata, > const struct samsung_retention_data *data); > diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c > index 963060920301..375634d8cc79 100644 > --- a/drivers/pinctrl/samsung/pinctrl-samsung.c > +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c > @@ -1349,6 +1349,9 @@ static int __maybe_unused samsung_pinctrl_suspend(struct device *dev) > const u8 *widths = bank->type->fld_width; > enum pincfg_type type; > > + if (drvdata->suspend) > + drvdata->suspend(bank); > + > /* Registers without a powerdown config aren't lost */ > if (!widths[PINCFG_TYPE_CON_PDN]) > continue; > @@ -1373,8 +1376,6 @@ static int __maybe_unused samsung_pinctrl_suspend(struct device *dev) > > clk_disable(drvdata->pclk); > > - if (drvdata->suspend) > - drvdata->suspend(drvdata); > if (drvdata->retention_ctrl && drvdata->retention_ctrl->enable) > drvdata->retention_ctrl->enable(drvdata); > > @@ -1406,9 +1407,6 @@ static int __maybe_unused samsung_pinctrl_resume(struct device *dev) > return ret; > } > > - if (drvdata->resume) > - drvdata->resume(drvdata); > - > for (i = 0; i < drvdata->nr_banks; i++) { > struct samsung_pin_bank *bank = &drvdata->pin_banks[i]; > void __iomem *reg = bank->pctl_base + bank->pctl_offset; > @@ -1416,6 +1414,9 @@ static int __maybe_unused samsung_pinctrl_resume(struct device *dev) > const u8 *widths = bank->type->fld_width; > enum pincfg_type type; > > + if (drvdata->resume) > + drvdata->resume(bank); > + > /* Registers without a powerdown config aren't lost */ > if (!widths[PINCFG_TYPE_CON_PDN]) > continue; > diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h b/drivers/pinctrl/samsung/pinctrl-samsung.h > index 371e4f02bbfb..e939e5bb0347 100644 > --- a/drivers/pinctrl/samsung/pinctrl-samsung.h > +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h > @@ -285,8 +285,8 @@ struct samsung_pin_ctrl { > int (*eint_gpio_init)(struct samsung_pinctrl_drv_data *); > int (*eint_wkup_init)(struct samsung_pinctrl_drv_data *); > void (*pud_value_init)(struct samsung_pinctrl_drv_data *drvdata); > - void (*suspend)(struct samsung_pinctrl_drv_data *); > - void (*resume)(struct samsung_pinctrl_drv_data *); > + void (*suspend)(struct samsung_pin_bank *bank); > + void (*resume)(struct samsung_pin_bank *bank); > }; > > /** > @@ -335,8 +335,8 @@ struct samsung_pinctrl_drv_data { > > struct samsung_retention_ctrl *retention_ctrl; > > - void (*suspend)(struct samsung_pinctrl_drv_data *); > - void (*resume)(struct samsung_pinctrl_drv_data *); > + void (*suspend)(struct samsung_pin_bank *bank); > + void (*resume)(struct samsung_pin_bank *bank); > }; > > /** > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/4] pinctrl: samsung: add dedicated SoC eint suspend/resume callbacks 2025-03-06 11:57 ` André Draszik @ 2025-03-06 15:12 ` André Draszik 2025-03-06 20:47 ` Peter Griffin 1 sibling, 0 replies; 11+ messages in thread From: André Draszik @ 2025-03-06 15:12 UTC (permalink / raw) To: Peter Griffin, Krzysztof Kozlowski, Sylwester Nawrocki, Alim Akhtar, Linus Walleij Cc: linux-arm-kernel, linux-samsung-soc, linux-gpio, linux-kernel, tudor.ambarus, willmcvicker, semen.protsenko, kernel-team, jaewon02.kim On Thu, 2025-03-06 at 11:57 +0000, André Draszik wrote: > On Sat, 2025-03-01 at 11:43 +0000, Peter Griffin wrote: > > [...] > > > > -static void exynos_pinctrl_resume_bank( > > - struct samsung_pinctrl_drv_data *drvdata, > > - struct samsung_pin_bank *bank) > > +void exynos_pinctrl_resume(struct samsung_pin_bank *bank) > > { > > struct exynos_eint_gpio_save *save = bank->soc_priv; > > void __iomem *regs = bank->eint_base; > > > > - if (clk_enable(bank->drvdata->pclk)) { > > - dev_err(bank->gpio_chip.parent, > > - "unable to enable clock for restoring state\n"); > > - return; > > + if (bank->eint_type == EINT_TYPE_GPIO) { > > + pr_debug("%s: con %#010x => %#010x\n", bank->name, > > + readl(regs + EXYNOS_GPIO_ECON_OFFSET > > + + bank->eint_offset), save->eint_con); > > + pr_debug("%s: fltcon0 %#010x => %#010x\n", bank->name, > > + readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > > + + 2 * bank->eint_offset), save->eint_fltcon0); > > + pr_debug("%s: fltcon1 %#010x => %#010x\n", bank->name, > > + readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > > + + 2 * bank->eint_offset + 4), > > + save->eint_fltcon1); The indent on this last line is a bit off. > > + pr_debug("%s: mask %#010x => %#010x\n", bank->name, > > + readl(regs + bank->irq_chip->eint_mask > > + + bank->eint_offset), save->eint_mask); > > + > > + writel(save->eint_con, regs + EXYNOS_GPIO_ECON_OFFSET > > + + bank->eint_offset); > > + writel(save->eint_fltcon0, regs + EXYNOS_GPIO_EFLTCON_OFFSET > > + + 2 * bank->eint_offset); > > + writel(save->eint_fltcon1, regs + EXYNOS_GPIO_EFLTCON_OFFSET > > + + 2 * bank->eint_offset + 4); > > + writel(save->eint_mask, regs + bank->irq_chip->eint_mask > > + + bank->eint_offset); > > } > > - > > - pr_debug("%s: con %#010x => %#010x\n", bank->name, > > - readl(regs + EXYNOS_GPIO_ECON_OFFSET > > - + bank->eint_offset), save->eint_con); > > - pr_debug("%s: fltcon0 %#010x => %#010x\n", bank->name, > > - readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > > - + 2 * bank->eint_offset), save->eint_fltcon0); > > - pr_debug("%s: fltcon1 %#010x => %#010x\n", bank->name, > > - readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > > - + 2 * bank->eint_offset + 4), save->eint_fltcon1); > > - pr_debug("%s: mask %#010x => %#010x\n", bank->name, > > - readl(regs + bank->irq_chip->eint_mask > > - + bank->eint_offset), save->eint_mask); > > - > > - writel(save->eint_con, regs + EXYNOS_GPIO_ECON_OFFSET > > - + bank->eint_offset); > > - writel(save->eint_fltcon0, regs + EXYNOS_GPIO_EFLTCON_OFFSET > > - + 2 * bank->eint_offset); > > - writel(save->eint_fltcon1, regs + EXYNOS_GPIO_EFLTCON_OFFSET > > - + 2 * bank->eint_offset + 4); > > - writel(save->eint_mask, regs + bank->irq_chip->eint_mask > > - + bank->eint_offset); > > - > > - clk_disable(bank->drvdata->pclk); > > } > > > > -static void exynosauto_pinctrl_resume_bank(struct samsung_pinctrl_drv_data *drvdata, > > - struct samsung_pin_bank *bank) > > +void exynosautov920_pinctrl_resume(struct samsung_pin_bank *bank) > > { > > struct exynos_eint_gpio_save *save = bank->soc_priv; > > void __iomem *regs = bank->eint_base; > > > > - if (clk_enable(bank->drvdata->pclk)) { > > - dev_err(bank->gpio_chip.parent, > > - "unable to enable clock for restoring state\n"); > > - return; > > + if (bank->eint_type == EINT_TYPE_GPIO) { > > + /* exynosautov920 has eint_con_offset for all but one bank */ > > + if (!bank->eint_con_offset) > > + exynos_pinctrl_resume(bank); > > + > > + pr_debug("%s: con %#010x => %#010x\n", bank->name, > > + readl(regs + bank->pctl_offset + bank->eint_con_offset), > > + save->eint_con); and here Cheers, Andre' ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/4] pinctrl: samsung: add dedicated SoC eint suspend/resume callbacks 2025-03-06 11:57 ` André Draszik 2025-03-06 15:12 ` André Draszik @ 2025-03-06 20:47 ` Peter Griffin 1 sibling, 0 replies; 11+ messages in thread From: Peter Griffin @ 2025-03-06 20:47 UTC (permalink / raw) To: André Draszik Cc: Krzysztof Kozlowski, Sylwester Nawrocki, Alim Akhtar, Linus Walleij, linux-arm-kernel, linux-samsung-soc, linux-gpio, linux-kernel, tudor.ambarus, willmcvicker, semen.protsenko, kernel-team, jaewon02.kim Hi André, Thanks for the review feedback :) On Thu, 6 Mar 2025 at 11:57, André Draszik <andre.draszik@linaro.org> wrote: > > On Sat, 2025-03-01 at 11:43 +0000, Peter Griffin wrote: > > gs101 needs it's own suspend/resume callbacks to use the newly > > added eint_fltcon_offset for saving & restoring fltcon0 & fltcon1 > > registers. It also differs to previous SoCs in that fltcon1 > > register doesn't always exist for each bank. > > > > exynosautov920 also has dedicated logic for using eint_con_offset > > and eint_mask_offset for saving & restoring it's registers. > > > > Refactor the existing platform specific suspend/resume callback > > so that each SoC variant has their own callback containing the > > SoC specific logic. > > > > Additionally we now call drvdata->suspend() & drvdata->resume() > > from within the loop that iterates the banks in > > samsung_pinctrl_suspend() and samsung_pinctrl_resume(). > > > > This simplifies the logic, and allows us to remove the > > clk_enable() and clk_disable() from the callbacks. > > > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > > --- > > Changes since v1: > > * Split code refactor & gs101 parts into separate patches (Andre) > > --- > > drivers/pinctrl/samsung/pinctrl-exynos-arm64.c | 28 ++-- > > drivers/pinctrl/samsung/pinctrl-exynos.c | 201 ++++++++++--------------- > > drivers/pinctrl/samsung/pinctrl-exynos.h | 6 +- > > drivers/pinctrl/samsung/pinctrl-samsung.c | 11 +- > > drivers/pinctrl/samsung/pinctrl-samsung.h | 8 +- > > 5 files changed, 111 insertions(+), 143 deletions(-) > > > > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c > > index e28fe8177646..57c98d2451b5 100644 > > --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c > > +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c > > @@ -1112,8 +1112,8 @@ static const struct samsung_pin_ctrl exynosautov920_pin_ctrl[] = { > > .pin_banks = exynosautov920_pin_banks0, > > .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks0), > > .eint_wkup_init = exynos_eint_wkup_init, > > - .suspend = exynos_pinctrl_suspend, > > - .resume = exynos_pinctrl_resume, > > + .suspend = exynosautov920_pinctrl_suspend, > > + .resume = exynosautov920_pinctrl_resume, > > .retention_data = &exynosautov920_retention_data, > > }, { > > /* pin-controller instance 1 AUD data */ > > @@ -1124,43 +1124,43 @@ static const struct samsung_pin_ctrl exynosautov920_pin_ctrl[] = { > > .pin_banks = exynosautov920_pin_banks2, > > .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks2), > > .eint_gpio_init = exynos_eint_gpio_init, > > - .suspend = exynos_pinctrl_suspend, > > - .resume = exynos_pinctrl_resume, > > + .suspend = exynosautov920_pinctrl_suspend, > > + .resume = exynosautov920_pinctrl_resume, > > }, { > > /* pin-controller instance 3 HSI1 data */ > > .pin_banks = exynosautov920_pin_banks3, > > .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks3), > > .eint_gpio_init = exynos_eint_gpio_init, > > - .suspend = exynos_pinctrl_suspend, > > - .resume = exynos_pinctrl_resume, > > + .suspend = exynosautov920_pinctrl_suspend, > > + .resume = exynosautov920_pinctrl_resume, > > }, { > > /* pin-controller instance 4 HSI2 data */ > > .pin_banks = exynosautov920_pin_banks4, > > .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks4), > > .eint_gpio_init = exynos_eint_gpio_init, > > - .suspend = exynos_pinctrl_suspend, > > - .resume = exynos_pinctrl_resume, > > + .suspend = exynosautov920_pinctrl_suspend, > > + .resume = exynosautov920_pinctrl_resume, > > }, { > > /* pin-controller instance 5 HSI2UFS data */ > > .pin_banks = exynosautov920_pin_banks5, > > .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks5), > > .eint_gpio_init = exynos_eint_gpio_init, > > - .suspend = exynos_pinctrl_suspend, > > - .resume = exynos_pinctrl_resume, > > + .suspend = exynosautov920_pinctrl_suspend, > > + .resume = exynosautov920_pinctrl_resume, > > }, { > > /* pin-controller instance 6 PERIC0 data */ > > .pin_banks = exynosautov920_pin_banks6, > > .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks6), > > .eint_gpio_init = exynos_eint_gpio_init, > > - .suspend = exynos_pinctrl_suspend, > > - .resume = exynos_pinctrl_resume, > > + .suspend = exynosautov920_pinctrl_suspend, > > + .resume = exynosautov920_pinctrl_resume, > > }, { > > /* pin-controller instance 7 PERIC1 data */ > > .pin_banks = exynosautov920_pin_banks7, > > .nr_banks = ARRAY_SIZE(exynosautov920_pin_banks7), > > .eint_gpio_init = exynos_eint_gpio_init, > > - .suspend = exynos_pinctrl_suspend, > > - .resume = exynos_pinctrl_resume, > > + .suspend = exynosautov920_pinctrl_suspend, > > + .resume = exynosautov920_pinctrl_resume, > > }, > > }; > > > > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c > > index ac6dc22b37c9..d65a9fba0781 100644 > > --- a/drivers/pinctrl/samsung/pinctrl-exynos.c > > +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c > > @@ -761,153 +761,118 @@ __init int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d) > > return 0; > > } > > > > -static void exynos_pinctrl_suspend_bank( > > - struct samsung_pinctrl_drv_data *drvdata, > > - struct samsung_pin_bank *bank) > > +static void exynos_set_wakeup(struct samsung_pin_bank *bank) > > { > > - struct exynos_eint_gpio_save *save = bank->soc_priv; > > - const void __iomem *regs = bank->eint_base; > > + struct exynos_irq_chip *irq_chip = NULL; > > This is a useless init and the value is overwritten right away. Will fix > > > > > - if (clk_enable(bank->drvdata->pclk)) { > > - dev_err(bank->gpio_chip.parent, > > - "unable to enable clock for saving state\n"); > > - return; > > + if (bank->eint_type == EINT_TYPE_WKUP) { > > + if (bank->irq_chip) { > > + irq_chip = bank->irq_chip; > > + irq_chip->set_eint_wakeup_mask(bank->drvdata, irq_chip); > > + } > > } > > - > > - save->eint_con = readl(regs + EXYNOS_GPIO_ECON_OFFSET > > - + bank->eint_offset); > > - save->eint_fltcon0 = readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > > - + 2 * bank->eint_offset); > > - save->eint_fltcon1 = readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > > - + 2 * bank->eint_offset + 4); > > - save->eint_mask = readl(regs + bank->irq_chip->eint_mask > > - + bank->eint_offset); > > - > > - clk_disable(bank->drvdata->pclk); > > - > > - pr_debug("%s: save con %#010x\n", bank->name, save->eint_con); > > - pr_debug("%s: save fltcon0 %#010x\n", bank->name, save->eint_fltcon0); > > - pr_debug("%s: save fltcon1 %#010x\n", bank->name, save->eint_fltcon1); > > - pr_debug("%s: save mask %#010x\n", bank->name, save->eint_mask); > > } > > > > -static void exynosauto_pinctrl_suspend_bank(struct samsung_pinctrl_drv_data *drvdata, > > - struct samsung_pin_bank *bank) > > +void exynos_pinctrl_suspend(struct samsung_pin_bank *bank) > > { > > struct exynos_eint_gpio_save *save = bank->soc_priv; > > const void __iomem *regs = bank->eint_base; > > > > - if (clk_enable(bank->drvdata->pclk)) { > > - dev_err(bank->gpio_chip.parent, > > - "unable to enable clock for saving state\n"); > > - return; > > + exynos_set_wakeup(bank); > > + > > + if (bank->eint_type == EINT_TYPE_GPIO) { > > Can you swap the order, and make the call to exynos_set_wakeup() > an else, to avoid some needless tests and make it clearer that > set_wakeup() is conditional as well, i.e.: > > if (bank->eint_type == EINT_TYPE_GPIO) { > ... > } else { > exynos_set_wakeup(bank); > } > > What makes it hard to read is that as-is, it looks like the > set_wakeup() is unconditional, while it's also based on > bank->eint_type (which is also more obvious before your patch). I've addressed this and the rest of your review feedback in the v3 which I just sent. Thanks, Peter. } } > > Other than that, I think I looks good. > > Cheers, > Andre' > > > + save->eint_con = readl(regs + EXYNOS_GPIO_ECON_OFFSET > > + + bank->eint_offset); > > + save->eint_fltcon0 = readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > > + + 2 * bank->eint_offset); > > + save->eint_fltcon1 = readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > > + + 2 * bank->eint_offset + 4); > > + save->eint_mask = readl(regs + bank->irq_chip->eint_mask > > + + bank->eint_offset); > > + > > + pr_debug("%s: save con %#010x\n", > > + bank->name, save->eint_con); > > + pr_debug("%s: save fltcon0 %#010x\n", > > + bank->name, save->eint_fltcon0); > > + pr_debug("%s: save fltcon1 %#010x\n", > > + bank->name, save->eint_fltcon1); > > + pr_debug("%s: save mask %#010x\n", > > + bank->name, save->eint_mask); > > } > > - > > - save->eint_con = readl(regs + bank->pctl_offset + bank->eint_con_offset); > > - save->eint_mask = readl(regs + bank->pctl_offset + bank->eint_mask_offset); > > - > > - clk_disable(bank->drvdata->pclk); > > - > > - pr_debug("%s: save con %#010x\n", bank->name, save->eint_con); > > - pr_debug("%s: save mask %#010x\n", bank->name, save->eint_mask); > > } > > > > -void exynos_pinctrl_suspend(struct samsung_pinctrl_drv_data *drvdata) > > +void exynosautov920_pinctrl_suspend(struct samsung_pin_bank *bank) > > { > > - struct samsung_pin_bank *bank = drvdata->pin_banks; > > - struct exynos_irq_chip *irq_chip = NULL; > > - int i; > > + struct exynos_eint_gpio_save *save = bank->soc_priv; > > + const void __iomem *regs = bank->eint_base; > > > > - for (i = 0; i < drvdata->nr_banks; ++i, ++bank) { > > - if (bank->eint_type == EINT_TYPE_GPIO) { > > - if (bank->eint_con_offset) > > - exynosauto_pinctrl_suspend_bank(drvdata, bank); > > - else > > - exynos_pinctrl_suspend_bank(drvdata, bank); > > - } > > - else if (bank->eint_type == EINT_TYPE_WKUP) { > > - if (!irq_chip) { > > - irq_chip = bank->irq_chip; > > - irq_chip->set_eint_wakeup_mask(drvdata, > > - irq_chip); > > - } > > - } > > + exynos_set_wakeup(bank); > > + > > + if (bank->eint_type == EINT_TYPE_GPIO) { > > + save->eint_con = readl(regs + bank->pctl_offset + > > + bank->eint_con_offset); > > + save->eint_mask = readl(regs + bank->pctl_offset + > > + bank->eint_mask_offset); > > + pr_debug("%s: save con %#010x\n", > > + bank->name, save->eint_con); > > + pr_debug("%s: save mask %#010x\n", > > + bank->name, save->eint_mask); > > } > > } > > > > -static void exynos_pinctrl_resume_bank( > > - struct samsung_pinctrl_drv_data *drvdata, > > - struct samsung_pin_bank *bank) > > +void exynos_pinctrl_resume(struct samsung_pin_bank *bank) > > { > > struct exynos_eint_gpio_save *save = bank->soc_priv; > > void __iomem *regs = bank->eint_base; > > > > - if (clk_enable(bank->drvdata->pclk)) { > > - dev_err(bank->gpio_chip.parent, > > - "unable to enable clock for restoring state\n"); > > - return; > > + if (bank->eint_type == EINT_TYPE_GPIO) { > > + pr_debug("%s: con %#010x => %#010x\n", bank->name, > > + readl(regs + EXYNOS_GPIO_ECON_OFFSET > > + + bank->eint_offset), save->eint_con); > > + pr_debug("%s: fltcon0 %#010x => %#010x\n", bank->name, > > + readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > > + + 2 * bank->eint_offset), save->eint_fltcon0); > > + pr_debug("%s: fltcon1 %#010x => %#010x\n", bank->name, > > + readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > > + + 2 * bank->eint_offset + 4), > > + save->eint_fltcon1); > > + pr_debug("%s: mask %#010x => %#010x\n", bank->name, > > + readl(regs + bank->irq_chip->eint_mask > > + + bank->eint_offset), save->eint_mask); > > + > > + writel(save->eint_con, regs + EXYNOS_GPIO_ECON_OFFSET > > + + bank->eint_offset); > > + writel(save->eint_fltcon0, regs + EXYNOS_GPIO_EFLTCON_OFFSET > > + + 2 * bank->eint_offset); > > + writel(save->eint_fltcon1, regs + EXYNOS_GPIO_EFLTCON_OFFSET > > + + 2 * bank->eint_offset + 4); > > + writel(save->eint_mask, regs + bank->irq_chip->eint_mask > > + + bank->eint_offset); > > } > > - > > - pr_debug("%s: con %#010x => %#010x\n", bank->name, > > - readl(regs + EXYNOS_GPIO_ECON_OFFSET > > - + bank->eint_offset), save->eint_con); > > - pr_debug("%s: fltcon0 %#010x => %#010x\n", bank->name, > > - readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > > - + 2 * bank->eint_offset), save->eint_fltcon0); > > - pr_debug("%s: fltcon1 %#010x => %#010x\n", bank->name, > > - readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET > > - + 2 * bank->eint_offset + 4), save->eint_fltcon1); > > - pr_debug("%s: mask %#010x => %#010x\n", bank->name, > > - readl(regs + bank->irq_chip->eint_mask > > - + bank->eint_offset), save->eint_mask); > > - > > - writel(save->eint_con, regs + EXYNOS_GPIO_ECON_OFFSET > > - + bank->eint_offset); > > - writel(save->eint_fltcon0, regs + EXYNOS_GPIO_EFLTCON_OFFSET > > - + 2 * bank->eint_offset); > > - writel(save->eint_fltcon1, regs + EXYNOS_GPIO_EFLTCON_OFFSET > > - + 2 * bank->eint_offset + 4); > > - writel(save->eint_mask, regs + bank->irq_chip->eint_mask > > - + bank->eint_offset); > > - > > - clk_disable(bank->drvdata->pclk); > > } > > > > -static void exynosauto_pinctrl_resume_bank(struct samsung_pinctrl_drv_data *drvdata, > > - struct samsung_pin_bank *bank) > > +void exynosautov920_pinctrl_resume(struct samsung_pin_bank *bank) > > { > > struct exynos_eint_gpio_save *save = bank->soc_priv; > > void __iomem *regs = bank->eint_base; > > > > - if (clk_enable(bank->drvdata->pclk)) { > > - dev_err(bank->gpio_chip.parent, > > - "unable to enable clock for restoring state\n"); > > - return; > > + if (bank->eint_type == EINT_TYPE_GPIO) { > > + /* exynosautov920 has eint_con_offset for all but one bank */ > > + if (!bank->eint_con_offset) > > + exynos_pinctrl_resume(bank); > > + > > + pr_debug("%s: con %#010x => %#010x\n", bank->name, > > + readl(regs + bank->pctl_offset + bank->eint_con_offset), > > + save->eint_con); > > + pr_debug("%s: mask %#010x => %#010x\n", bank->name, > > + readl(regs + bank->pctl_offset + > > + bank->eint_mask_offset), save->eint_mask); > > + > > + writel(save->eint_con, > > + regs + bank->pctl_offset + bank->eint_con_offset); > > + writel(save->eint_mask, > > + regs + bank->pctl_offset + bank->eint_mask_offset); > > } > > - > > - pr_debug("%s: con %#010x => %#010x\n", bank->name, > > - readl(regs + bank->pctl_offset + bank->eint_con_offset), save->eint_con); > > - pr_debug("%s: mask %#010x => %#010x\n", bank->name, > > - readl(regs + bank->pctl_offset + bank->eint_mask_offset), save->eint_mask); > > - > > - writel(save->eint_con, regs + bank->pctl_offset + bank->eint_con_offset); > > - writel(save->eint_mask, regs + bank->pctl_offset + bank->eint_mask_offset); > > - > > - clk_disable(bank->drvdata->pclk); > > -} > > - > > -void exynos_pinctrl_resume(struct samsung_pinctrl_drv_data *drvdata) > > -{ > > - struct samsung_pin_bank *bank = drvdata->pin_banks; > > - int i; > > - > > - for (i = 0; i < drvdata->nr_banks; ++i, ++bank) > > - if (bank->eint_type == EINT_TYPE_GPIO) { > > - if (bank->eint_con_offset) > > - exynosauto_pinctrl_resume_bank(drvdata, bank); > > - else > > - exynos_pinctrl_resume_bank(drvdata, bank); > > - } > > } > > > > 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 33df21d5c9d6..35c2bc4ea488 100644 > > --- a/drivers/pinctrl/samsung/pinctrl-exynos.h > > +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h > > @@ -221,8 +221,10 @@ struct exynos_muxed_weint_data { > > > > int exynos_eint_gpio_init(struct samsung_pinctrl_drv_data *d); > > int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d); > > -void exynos_pinctrl_suspend(struct samsung_pinctrl_drv_data *drvdata); > > -void exynos_pinctrl_resume(struct samsung_pinctrl_drv_data *drvdata); > > +void exynosautov920_pinctrl_resume(struct samsung_pin_bank *bank); > > +void exynosautov920_pinctrl_suspend(struct samsung_pin_bank *bank); > > +void exynos_pinctrl_suspend(struct samsung_pin_bank *bank); > > +void exynos_pinctrl_resume(struct samsung_pin_bank *bank); > > struct samsung_retention_ctrl * > > exynos_retention_init(struct samsung_pinctrl_drv_data *drvdata, > > const struct samsung_retention_data *data); > > diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c > > index 963060920301..375634d8cc79 100644 > > --- a/drivers/pinctrl/samsung/pinctrl-samsung.c > > +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c > > @@ -1349,6 +1349,9 @@ static int __maybe_unused samsung_pinctrl_suspend(struct device *dev) > > const u8 *widths = bank->type->fld_width; > > enum pincfg_type type; > > > > + if (drvdata->suspend) > > + drvdata->suspend(bank); > > + > > /* Registers without a powerdown config aren't lost */ > > if (!widths[PINCFG_TYPE_CON_PDN]) > > continue; > > @@ -1373,8 +1376,6 @@ static int __maybe_unused samsung_pinctrl_suspend(struct device *dev) > > > > clk_disable(drvdata->pclk); > > > > - if (drvdata->suspend) > > - drvdata->suspend(drvdata); > > if (drvdata->retention_ctrl && drvdata->retention_ctrl->enable) > > drvdata->retention_ctrl->enable(drvdata); > > > > @@ -1406,9 +1407,6 @@ static int __maybe_unused samsung_pinctrl_resume(struct device *dev) > > return ret; > > } > > > > - if (drvdata->resume) > > - drvdata->resume(drvdata); > > - > > for (i = 0; i < drvdata->nr_banks; i++) { > > struct samsung_pin_bank *bank = &drvdata->pin_banks[i]; > > void __iomem *reg = bank->pctl_base + bank->pctl_offset; > > @@ -1416,6 +1414,9 @@ static int __maybe_unused samsung_pinctrl_resume(struct device *dev) > > const u8 *widths = bank->type->fld_width; > > enum pincfg_type type; > > > > + if (drvdata->resume) > > + drvdata->resume(bank); > > + > > /* Registers without a powerdown config aren't lost */ > > if (!widths[PINCFG_TYPE_CON_PDN]) > > continue; > > diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h b/drivers/pinctrl/samsung/pinctrl-samsung.h > > index 371e4f02bbfb..e939e5bb0347 100644 > > --- a/drivers/pinctrl/samsung/pinctrl-samsung.h > > +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h > > @@ -285,8 +285,8 @@ struct samsung_pin_ctrl { > > int (*eint_gpio_init)(struct samsung_pinctrl_drv_data *); > > int (*eint_wkup_init)(struct samsung_pinctrl_drv_data *); > > void (*pud_value_init)(struct samsung_pinctrl_drv_data *drvdata); > > - void (*suspend)(struct samsung_pinctrl_drv_data *); > > - void (*resume)(struct samsung_pinctrl_drv_data *); > > + void (*suspend)(struct samsung_pin_bank *bank); > > + void (*resume)(struct samsung_pin_bank *bank); > > }; > > > > /** > > @@ -335,8 +335,8 @@ struct samsung_pinctrl_drv_data { > > > > struct samsung_retention_ctrl *retention_ctrl; > > > > - void (*suspend)(struct samsung_pinctrl_drv_data *); > > - void (*resume)(struct samsung_pinctrl_drv_data *); > > + void (*suspend)(struct samsung_pin_bank *bank); > > + void (*resume)(struct samsung_pin_bank *bank); > > }; > > > > /** > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 3/4] pinctrl: samsung: add gs101 specific eint suspend/resume callbacks 2025-03-01 11:43 [PATCH v2 0/4] samsung: pinctrl: Add support for eint_fltcon_offset and filter selection on gs101 Peter Griffin 2025-03-01 11:43 ` [PATCH v2 1/4] pinctrl: samsung: add support for eint_fltcon_offset Peter Griffin 2025-03-01 11:43 ` [PATCH v2 2/4] pinctrl: samsung: add dedicated SoC eint suspend/resume callbacks Peter Griffin @ 2025-03-01 11:43 ` Peter Griffin 2025-03-06 15:12 ` André Draszik 2025-03-01 11:43 ` [PATCH v2 4/4] pinctrl: samsung: Add filter selection support for alive bank on gs101 Peter Griffin 3 siblings, 1 reply; 11+ messages in thread From: Peter Griffin @ 2025-03-01 11:43 UTC (permalink / raw) To: Krzysztof Kozlowski, Sylwester Nawrocki, Alim Akhtar, Linus Walleij Cc: linux-arm-kernel, linux-samsung-soc, linux-gpio, linux-kernel, andre.draszik, tudor.ambarus, willmcvicker, semen.protsenko, kernel-team, jaewon02.kim, Peter Griffin, stable gs101 differs to other SoCs in that fltcon1 register doesn't always exist. Additionally the offset of fltcon0 is not fixed and needs to use the newly added eint_fltcon_offset variable. Signed-off-by: Peter Griffin <peter.griffin@linaro.org> Fixes: 4a8be01a1a7a ("pinctrl: samsung: Add gs101 SoC pinctrl configuration") Cc: stable@vger.kernel.org --- drivers/pinctrl/samsung/pinctrl-exynos-arm64.c | 24 ++++----- drivers/pinctrl/samsung/pinctrl-exynos.c | 71 ++++++++++++++++++++++++++ drivers/pinctrl/samsung/pinctrl-exynos.h | 2 + 3 files changed, 85 insertions(+), 12 deletions(-) diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c index 57c98d2451b5..fca447ebc5f5 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c @@ -1455,15 +1455,15 @@ static const struct samsung_pin_ctrl gs101_pin_ctrl[] __initconst = { .pin_banks = gs101_pin_alive, .nr_banks = ARRAY_SIZE(gs101_pin_alive), .eint_wkup_init = exynos_eint_wkup_init, - .suspend = exynos_pinctrl_suspend, - .resume = exynos_pinctrl_resume, + .suspend = gs101_pinctrl_suspend, + .resume = gs101_pinctrl_resume, }, { /* pin banks of gs101 pin-controller (FAR_ALIVE) */ .pin_banks = gs101_pin_far_alive, .nr_banks = ARRAY_SIZE(gs101_pin_far_alive), .eint_wkup_init = exynos_eint_wkup_init, - .suspend = exynos_pinctrl_suspend, - .resume = exynos_pinctrl_resume, + .suspend = gs101_pinctrl_suspend, + .resume = gs101_pinctrl_resume, }, { /* pin banks of gs101 pin-controller (GSACORE) */ .pin_banks = gs101_pin_gsacore, @@ -1477,29 +1477,29 @@ static const struct samsung_pin_ctrl gs101_pin_ctrl[] __initconst = { .pin_banks = gs101_pin_peric0, .nr_banks = ARRAY_SIZE(gs101_pin_peric0), .eint_gpio_init = exynos_eint_gpio_init, - .suspend = exynos_pinctrl_suspend, - .resume = exynos_pinctrl_resume, + .suspend = gs101_pinctrl_suspend, + .resume = gs101_pinctrl_resume, }, { /* pin banks of gs101 pin-controller (PERIC1) */ .pin_banks = gs101_pin_peric1, .nr_banks = ARRAY_SIZE(gs101_pin_peric1), .eint_gpio_init = exynos_eint_gpio_init, - .suspend = exynos_pinctrl_suspend, - .resume = exynos_pinctrl_resume, + .suspend = gs101_pinctrl_suspend, + .resume = gs101_pinctrl_resume, }, { /* pin banks of gs101 pin-controller (HSI1) */ .pin_banks = gs101_pin_hsi1, .nr_banks = ARRAY_SIZE(gs101_pin_hsi1), .eint_gpio_init = exynos_eint_gpio_init, - .suspend = exynos_pinctrl_suspend, - .resume = exynos_pinctrl_resume, + .suspend = gs101_pinctrl_suspend, + .resume = gs101_pinctrl_resume, }, { /* pin banks of gs101 pin-controller (HSI2) */ .pin_banks = gs101_pin_hsi2, .nr_banks = ARRAY_SIZE(gs101_pin_hsi2), .eint_gpio_init = exynos_eint_gpio_init, - .suspend = exynos_pinctrl_suspend, - .resume = exynos_pinctrl_resume, + .suspend = gs101_pinctrl_suspend, + .resume = gs101_pinctrl_resume, }, }; diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c index d65a9fba0781..ddc7245ec2e5 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c @@ -801,6 +801,41 @@ void exynos_pinctrl_suspend(struct samsung_pin_bank *bank) } } +void gs101_pinctrl_suspend(struct samsung_pin_bank *bank) +{ + struct exynos_eint_gpio_save *save = bank->soc_priv; + const void __iomem *regs = bank->eint_base; + + exynos_set_wakeup(bank); + + if (bank->eint_type == EINT_TYPE_GPIO) { + save->eint_con = readl(regs + EXYNOS_GPIO_ECON_OFFSET + + bank->eint_offset); + + save->eint_fltcon0 = readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET + + bank->eint_fltcon_offset); + + /* fltcon1 register only exists for pins 4-7 */ + if (bank->nr_pins > 4) + save->eint_fltcon1 = readl(regs + + EXYNOS_GPIO_EFLTCON_OFFSET + + bank->eint_fltcon_offset + 4); + + save->eint_mask = readl(regs + bank->irq_chip->eint_mask + + bank->eint_offset); + + pr_debug("%s: save con %#010x\n", + bank->name, save->eint_con); + pr_debug("%s: save fltcon0 %#010x\n", + bank->name, save->eint_fltcon0); + if (bank->nr_pins > 4) + pr_debug("%s: save fltcon1 %#010x\n", + bank->name, save->eint_fltcon1); + pr_debug("%s: save mask %#010x\n", + bank->name, save->eint_mask); + } +} + void exynosautov920_pinctrl_suspend(struct samsung_pin_bank *bank) { struct exynos_eint_gpio_save *save = bank->soc_priv; @@ -820,6 +855,42 @@ void exynosautov920_pinctrl_suspend(struct samsung_pin_bank *bank) } } +void gs101_pinctrl_resume(struct samsung_pin_bank *bank) +{ + struct exynos_eint_gpio_save *save = bank->soc_priv; + + void __iomem *regs = bank->eint_base; + void __iomem *eint_fltcfg0 = regs + EXYNOS_GPIO_EFLTCON_OFFSET + + bank->eint_fltcon_offset; + + if (bank->eint_type == EINT_TYPE_GPIO) { + pr_debug("%s: con %#010x => %#010x\n", bank->name, + readl(regs + EXYNOS_GPIO_ECON_OFFSET + + bank->eint_offset), save->eint_con); + + pr_debug("%s: fltcon0 %#010x => %#010x\n", bank->name, + readl(eint_fltcfg0), save->eint_fltcon0); + + /* fltcon1 register only exists for pins 4-7 */ + if (bank->nr_pins > 4) { + pr_debug("%s: fltcon1 %#010x => %#010x\n", bank->name, + readl(eint_fltcfg0 + 4), save->eint_fltcon1); + } + pr_debug("%s: mask %#010x => %#010x\n", bank->name, + readl(regs + bank->irq_chip->eint_mask + + bank->eint_offset), save->eint_mask); + + writel(save->eint_con, regs + EXYNOS_GPIO_ECON_OFFSET + + bank->eint_offset); + writel(save->eint_fltcon0, eint_fltcfg0); + + if (bank->nr_pins > 4) + writel(save->eint_fltcon1, eint_fltcfg0 + 4); + writel(save->eint_mask, regs + bank->irq_chip->eint_mask + + bank->eint_offset); + } +} + void exynos_pinctrl_resume(struct samsung_pin_bank *bank) { struct exynos_eint_gpio_save *save = bank->soc_priv; diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h b/drivers/pinctrl/samsung/pinctrl-exynos.h index 35c2bc4ea488..773f161a82a3 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.h +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h @@ -225,6 +225,8 @@ void exynosautov920_pinctrl_resume(struct samsung_pin_bank *bank); void exynosautov920_pinctrl_suspend(struct samsung_pin_bank *bank); void exynos_pinctrl_suspend(struct samsung_pin_bank *bank); void exynos_pinctrl_resume(struct samsung_pin_bank *bank); +void gs101_pinctrl_suspend(struct samsung_pin_bank *bank); +void gs101_pinctrl_resume(struct samsung_pin_bank *bank); struct samsung_retention_ctrl * exynos_retention_init(struct samsung_pinctrl_drv_data *drvdata, const struct samsung_retention_data *data); -- 2.48.1.711.g2feabab25a-goog ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/4] pinctrl: samsung: add gs101 specific eint suspend/resume callbacks 2025-03-01 11:43 ` [PATCH v2 3/4] pinctrl: samsung: add gs101 specific " Peter Griffin @ 2025-03-06 15:12 ` André Draszik 0 siblings, 0 replies; 11+ messages in thread From: André Draszik @ 2025-03-06 15:12 UTC (permalink / raw) To: Peter Griffin, Krzysztof Kozlowski, Sylwester Nawrocki, Alim Akhtar, Linus Walleij Cc: linux-arm-kernel, linux-samsung-soc, linux-gpio, linux-kernel, tudor.ambarus, willmcvicker, semen.protsenko, kernel-team, jaewon02.kim, stable On Sat, 2025-03-01 at 11:43 +0000, Peter Griffin wrote: > gs101 differs to other SoCs in that fltcon1 register doesn't > always exist. Additionally the offset of fltcon0 is not fixed > and needs to use the newly added eint_fltcon_offset variable. > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > Fixes: 4a8be01a1a7a ("pinctrl: samsung: Add gs101 SoC pinctrl configuration") > Cc: stable@vger.kernel.org > --- > drivers/pinctrl/samsung/pinctrl-exynos-arm64.c | 24 ++++----- > drivers/pinctrl/samsung/pinctrl-exynos.c | 71 ++++++++++++++++++++++++++ > drivers/pinctrl/samsung/pinctrl-exynos.h | 2 + > 3 files changed, 85 insertions(+), 12 deletions(-) > > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c > index 57c98d2451b5..fca447ebc5f5 100644 > --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c > +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c > @@ -1455,15 +1455,15 @@ static const struct samsung_pin_ctrl gs101_pin_ctrl[] __initconst = { > .pin_banks = gs101_pin_alive, > .nr_banks = ARRAY_SIZE(gs101_pin_alive), > .eint_wkup_init = exynos_eint_wkup_init, > - .suspend = exynos_pinctrl_suspend, > - .resume = exynos_pinctrl_resume, > + .suspend = gs101_pinctrl_suspend, > + .resume = gs101_pinctrl_resume, > }, { > /* pin banks of gs101 pin-controller (FAR_ALIVE) */ > .pin_banks = gs101_pin_far_alive, > .nr_banks = ARRAY_SIZE(gs101_pin_far_alive), > .eint_wkup_init = exynos_eint_wkup_init, > - .suspend = exynos_pinctrl_suspend, > - .resume = exynos_pinctrl_resume, > + .suspend = gs101_pinctrl_suspend, > + .resume = gs101_pinctrl_resume, > }, { > /* pin banks of gs101 pin-controller (GSACORE) */ > .pin_banks = gs101_pin_gsacore, > @@ -1477,29 +1477,29 @@ static const struct samsung_pin_ctrl gs101_pin_ctrl[] __initconst = { > .pin_banks = gs101_pin_peric0, > .nr_banks = ARRAY_SIZE(gs101_pin_peric0), > .eint_gpio_init = exynos_eint_gpio_init, > - .suspend = exynos_pinctrl_suspend, > - .resume = exynos_pinctrl_resume, > + .suspend = gs101_pinctrl_suspend, > + .resume = gs101_pinctrl_resume, > }, { > /* pin banks of gs101 pin-controller (PERIC1) */ > .pin_banks = gs101_pin_peric1, > .nr_banks = ARRAY_SIZE(gs101_pin_peric1), > .eint_gpio_init = exynos_eint_gpio_init, > - .suspend = exynos_pinctrl_suspend, > - .resume = exynos_pinctrl_resume, > + .suspend = gs101_pinctrl_suspend, > + .resume = gs101_pinctrl_resume, > }, { > /* pin banks of gs101 pin-controller (HSI1) */ > .pin_banks = gs101_pin_hsi1, > .nr_banks = ARRAY_SIZE(gs101_pin_hsi1), > .eint_gpio_init = exynos_eint_gpio_init, > - .suspend = exynos_pinctrl_suspend, > - .resume = exynos_pinctrl_resume, > + .suspend = gs101_pinctrl_suspend, > + .resume = gs101_pinctrl_resume, > }, { > /* pin banks of gs101 pin-controller (HSI2) */ > .pin_banks = gs101_pin_hsi2, > .nr_banks = ARRAY_SIZE(gs101_pin_hsi2), > .eint_gpio_init = exynos_eint_gpio_init, > - .suspend = exynos_pinctrl_suspend, > - .resume = exynos_pinctrl_resume, > + .suspend = gs101_pinctrl_suspend, > + .resume = gs101_pinctrl_resume, > }, > }; > > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c > index d65a9fba0781..ddc7245ec2e5 100644 > --- a/drivers/pinctrl/samsung/pinctrl-exynos.c > +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c > @@ -801,6 +801,41 @@ void exynos_pinctrl_suspend(struct samsung_pin_bank *bank) > } > } > > +void gs101_pinctrl_suspend(struct samsung_pin_bank *bank) > +{ > + struct exynos_eint_gpio_save *save = bank->soc_priv; > + const void __iomem *regs = bank->eint_base; > + > + exynos_set_wakeup(bank); As for patch 2, would be good to have this if / else, to make it more obvious that this is conditional, too. > + > + if (bank->eint_type == EINT_TYPE_GPIO) { > + save->eint_con = readl(regs + EXYNOS_GPIO_ECON_OFFSET > + + bank->eint_offset); > + > + save->eint_fltcon0 = readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET + > + bank->eint_fltcon_offset); If there's another version, maybe align style where the '+' is placed (end of line vs start of line). It seems this file generally uses + at start of line. > + > + /* fltcon1 register only exists for pins 4-7 */ > + if (bank->nr_pins > 4) > + save->eint_fltcon1 = readl(regs + > + EXYNOS_GPIO_EFLTCON_OFFSET + > + bank->eint_fltcon_offset + 4); > + > + save->eint_mask = readl(regs + bank->irq_chip->eint_mask > + + bank->eint_offset); > + > + pr_debug("%s: save con %#010x\n", > + bank->name, save->eint_con); > + pr_debug("%s: save fltcon0 %#010x\n", > + bank->name, save->eint_fltcon0); > + if (bank->nr_pins > 4) > + pr_debug("%s: save fltcon1 %#010x\n", > + bank->name, save->eint_fltcon1); > + pr_debug("%s: save mask %#010x\n", > + bank->name, save->eint_mask); > + } > +} > + > void exynosautov920_pinctrl_suspend(struct samsung_pin_bank *bank) > { > struct exynos_eint_gpio_save *save = bank->soc_priv; > @@ -820,6 +855,42 @@ void exynosautov920_pinctrl_suspend(struct samsung_pin_bank *bank) > } > } > > +void gs101_pinctrl_resume(struct samsung_pin_bank *bank) > +{ > + struct exynos_eint_gpio_save *save = bank->soc_priv; > + > + void __iomem *regs = bank->eint_base; > + void __iomem *eint_fltcfg0 = regs + EXYNOS_GPIO_EFLTCON_OFFSET > + + bank->eint_fltcon_offset; > + > + if (bank->eint_type == EINT_TYPE_GPIO) { > + pr_debug("%s: con %#010x => %#010x\n", bank->name, > + readl(regs + EXYNOS_GPIO_ECON_OFFSET > + + bank->eint_offset), save->eint_con); > + > + pr_debug("%s: fltcon0 %#010x => %#010x\n", bank->name, > + readl(eint_fltcfg0), save->eint_fltcon0); > + > + /* fltcon1 register only exists for pins 4-7 */ > + if (bank->nr_pins > 4) { > + pr_debug("%s: fltcon1 %#010x => %#010x\n", bank->name, > + readl(eint_fltcfg0 + 4), save->eint_fltcon1); > + } If there's another version, braces are not needed here. Cheers, Andre' > + pr_debug("%s: mask %#010x => %#010x\n", bank->name, > + readl(regs + bank->irq_chip->eint_mask > + + bank->eint_offset), save->eint_mask); > + > + writel(save->eint_con, regs + EXYNOS_GPIO_ECON_OFFSET > + + bank->eint_offset); > + writel(save->eint_fltcon0, eint_fltcfg0); > + > + if (bank->nr_pins > 4) > + writel(save->eint_fltcon1, eint_fltcfg0 + 4); > + writel(save->eint_mask, regs + bank->irq_chip->eint_mask > + + bank->eint_offset); > + } > +} > + > void exynos_pinctrl_resume(struct samsung_pin_bank *bank) > { > struct exynos_eint_gpio_save *save = bank->soc_priv; > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h b/drivers/pinctrl/samsung/pinctrl-exynos.h > index 35c2bc4ea488..773f161a82a3 100644 > --- a/drivers/pinctrl/samsung/pinctrl-exynos.h > +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h > @@ -225,6 +225,8 @@ void exynosautov920_pinctrl_resume(struct samsung_pin_bank *bank); > void exynosautov920_pinctrl_suspend(struct samsung_pin_bank *bank); > void exynos_pinctrl_suspend(struct samsung_pin_bank *bank); > void exynos_pinctrl_resume(struct samsung_pin_bank *bank); > +void gs101_pinctrl_suspend(struct samsung_pin_bank *bank); > +void gs101_pinctrl_resume(struct samsung_pin_bank *bank); > struct samsung_retention_ctrl * > exynos_retention_init(struct samsung_pinctrl_drv_data *drvdata, > const struct samsung_retention_data *data); > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 4/4] pinctrl: samsung: Add filter selection support for alive bank on gs101 2025-03-01 11:43 [PATCH v2 0/4] samsung: pinctrl: Add support for eint_fltcon_offset and filter selection on gs101 Peter Griffin ` (2 preceding siblings ...) 2025-03-01 11:43 ` [PATCH v2 3/4] pinctrl: samsung: add gs101 specific " Peter Griffin @ 2025-03-01 11:43 ` Peter Griffin 2025-03-06 15:26 ` André Draszik 3 siblings, 1 reply; 11+ messages in thread From: Peter Griffin @ 2025-03-01 11:43 UTC (permalink / raw) To: Krzysztof Kozlowski, Sylwester Nawrocki, Alim Akhtar, Linus Walleij Cc: linux-arm-kernel, linux-samsung-soc, linux-gpio, linux-kernel, andre.draszik, tudor.ambarus, willmcvicker, semen.protsenko, kernel-team, jaewon02.kim, Peter Griffin 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> --- 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; + } + 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); } 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); } 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, \ -- 2.48.1.711.g2feabab25a-goog ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 4/4] pinctrl: samsung: Add filter selection support for alive bank on gs101 2025-03-01 11:43 ` [PATCH v2 4/4] pinctrl: samsung: Add filter selection support for alive bank on gs101 Peter Griffin @ 2025-03-06 15:26 ` André Draszik 0 siblings, 0 replies; 11+ messages in thread From: André Draszik @ 2025-03-06 15:26 UTC (permalink / raw) To: Peter Griffin, Krzysztof Kozlowski, Sylwester Nawrocki, Alim Akhtar, Linus Walleij Cc: linux-arm-kernel, linux-samsung-soc, linux-gpio, linux-kernel, tudor.ambarus, willmcvicker, semen.protsenko, kernel-team, jaewon02.kim 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@linaro.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, \ > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-03-06 20:47 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-03-01 11:43 [PATCH v2 0/4] samsung: pinctrl: Add support for eint_fltcon_offset and filter selection on gs101 Peter Griffin 2025-03-01 11:43 ` [PATCH v2 1/4] pinctrl: samsung: add support for eint_fltcon_offset Peter Griffin 2025-03-06 11:24 ` André Draszik 2025-03-01 11:43 ` [PATCH v2 2/4] pinctrl: samsung: add dedicated SoC eint suspend/resume callbacks Peter Griffin 2025-03-06 11:57 ` André Draszik 2025-03-06 15:12 ` André Draszik 2025-03-06 20:47 ` Peter Griffin 2025-03-01 11:43 ` [PATCH v2 3/4] pinctrl: samsung: add gs101 specific " Peter Griffin 2025-03-06 15:12 ` André Draszik 2025-03-01 11:43 ` [PATCH v2 4/4] pinctrl: samsung: Add filter selection support for alive bank on gs101 Peter Griffin 2025-03-06 15:26 ` André Draszik
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).