[PATCH v4 0/4] samsung: pinctrl: Add support for eint_fltcon_offset and filter selection on gs101
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 v4: - save->eint_fltcon1 is an argument to pr_debug(), not readl() change alignment accordingly (Andre) - Link to v3: https://lore.kernel.org/r/20250306-pinctrl-fltcon-suspend-v3-0-f9ab4ff6a24e@...
Changes in v3: - Ensure EXYNOS_FLTCON_DIGITAL bit is cleared (Andre) - Make it obvious that exynos_eint_set_filter() is conditional on bank type (Andre) - Make it obvious exynos_set_wakeup() is conditional on bank type (Andre) - Align style where the '+' is placed first (Andre) - Remove unnecessary braces (Andre) - Link to v2: https://lore.kernel.org/r/20250301-pinctrl-fltcon-suspend-v2-0-a7eef9bb443b@...
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@...
--- 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 | 294 +++++++++++++++---------- drivers/pinctrl/samsung/pinctrl-exynos.h | 50 ++++- drivers/pinctrl/samsung/pinctrl-samsung.c | 12 +- drivers/pinctrl/samsung/pinctrl-samsung.h | 12 +- 5 files changed, 318 insertions(+), 200 deletions(-) --- base-commit: 0761652a3b3b607787aebc386d412b1d0ae8008c change-id: 20250120-pinctrl-fltcon-suspend-2333a137c4d4
Best regards,
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.
Fixes: 4a8be01a1a7a ("pinctrl: samsung: Add gs101 SoC pinctrl configuration") Cc: stable@vger.kernel.org Reviewed-by: André Draszik andre.draszik@linaro.org Signed-off-by: Peter Griffin peter.griffin@linaro.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 3ea7106ce5eae3c21f11790b5a40037042c1d407..e28fe81776466b693417c66bb15752d609b79eb1 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 7b7ff7ffeb56bdde2504ec90c8df13bdd1ace70e..33df21d5c9d61e852834031570d4a0ac0e51f6a4 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 cfced7afd4ca6e99c7e72a588d780f0542990ed9..963060920301ec90affb2ee6d758d3d602ffb4a9 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 bb0689d52ea0b4392714fa9bcdcbae8d253c73a1..371e4f02bbfb375964b7833beb9bbc098a51f4a3 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;
On Fri, 07 Mar 2025 10:29:05 +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.
[...]
Re-wrapped commit msg to match wrapping style.
Applied, thanks!
[1/4] pinctrl: samsung: add support for eint_fltcon_offset https://git.kernel.org/pinctrl/samsung/c/701d0e910955627734917c3587258aa7e73...
Best regards,
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.
Fixes: 4a8be01a1a7a ("pinctrl: samsung: Add gs101 SoC pinctrl configuration") Cc: stable@vger.kernel.org Reviewed-by: André Draszik andre.draszik@linaro.org Signed-off-by: Peter Griffin peter.griffin@linaro.org --- Changes since v2: * make it clear exynos_set_wakeup(bank) is conditional on bank type (Andre) * align style where the '+' is placed (Andre) * remove unnecessary braces (Andre) --- drivers/pinctrl/samsung/pinctrl-exynos-arm64.c | 24 ++++----- drivers/pinctrl/samsung/pinctrl-exynos.c | 70 ++++++++++++++++++++++++++ drivers/pinctrl/samsung/pinctrl-exynos.h | 2 + 3 files changed, 84 insertions(+), 12 deletions(-)
diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c index 57c98d2451b54b00d50e0e948e272ed53d386c34..fca447ebc5f5956b7e8d2f2d08f23622095b1ee6 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 75b9ab19e4e8f81bf85cd75573485b7f2e717e7b..5f0045d03346600557fa6735bad709897c71935c 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c @@ -798,6 +798,40 @@ void exynos_pinctrl_suspend(struct samsung_pin_bank *bank) exynos_set_wakeup(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; + + 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); + } else if (bank->eint_type == EINT_TYPE_WKUP) + exynos_set_wakeup(bank); +} + void exynosautov920_pinctrl_suspend(struct samsung_pin_bank *bank) { struct exynos_eint_gpio_save *save = bank->soc_priv; @@ -816,6 +850,42 @@ void exynosautov920_pinctrl_suspend(struct samsung_pin_bank *bank) exynos_set_wakeup(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 35c2bc4ea488bda600ebfbda1492f5f49dbd9849..773f161a82a38cbaad05fcbc09a936300f5c7595 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);
On 07/03/2025 11:29, 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.
Fixes: 4a8be01a1a7a ("pinctrl: samsung: Add gs101 SoC pinctrl configuration") Cc: stable@vger.kernel.org
It looks this depends on previous commit, right? That's really not optimal, although I understand that if you re-order patches this code would be soon changed, just like you changed other suspend/resume callbacks in patch #2?
Reviewed-by: André Draszik andre.draszik@linaro.org Signed-off-by: Peter Griffin peter.griffin@linaro.org
Changes since v2:
- make it clear exynos_set_wakeup(bank) is conditional on bank type (Andre)
- align style where the '+' is placed (Andre)
- remove unnecessary braces (Andre)
...
+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;
- 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);- } else if (bank->eint_type == EINT_TYPE_WKUP)
exynos_set_wakeup(bank);
Missing {}. Run checkpatch --strict.
Best regards, Krzysztof
Hi Krzysztof,
Thanks for the review feedback.
On Tue, 11 Mar 2025 at 19:36, Krzysztof Kozlowski krzk@kernel.org wrote:
On 07/03/2025 11:29, 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.
Fixes: 4a8be01a1a7a ("pinctrl: samsung: Add gs101 SoC pinctrl configuration") Cc: stable@vger.kernel.org
It looks this depends on previous commit, right?
Yes that's right, it depends on the refactoring in the previous patch. To fix the bug (which is an Serror on suspend for gs101), we need the dedicated gs101 callback so it can have the knowledge that fltcon1 doesn't always exist and it's varying offset.
That's really not optimal, although I understand that if you re-order patches this code would be soon changed, just like you changed other suspend/resume callbacks in patch #2?
Originally it was just one patch, but the previous review feedback was to split the refactor into a dedicated patch, and then add gs101 specific parts separately. The refactoring was done primarily so we can fix this bug without affecting the other platforms, so I don't re-ordering the patches will help.
Thanks,
Peter
Reviewed-by: André Draszik andre.draszik@linaro.org Signed-off-by: Peter Griffin peter.griffin@linaro.org
Changes since v2:
- make it clear exynos_set_wakeup(bank) is conditional on bank type (Andre)
- align style where the '+' is placed (Andre)
- remove unnecessary braces (Andre)
...
+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;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);} else if (bank->eint_type == EINT_TYPE_WKUP)exynos_set_wakeup(bank);Missing {}. Run checkpatch --strict.
Best regards, Krzysztof
Hi Krzysztof,
On Wed, 12 Mar 2025 at 11:31, Peter Griffin peter.griffin@linaro.org wrote:
Hi Krzysztof,
Thanks for the review feedback.
On Tue, 11 Mar 2025 at 19:36, Krzysztof Kozlowski krzk@kernel.org wrote:
On 07/03/2025 11:29, 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.
Fixes: 4a8be01a1a7a ("pinctrl: samsung: Add gs101 SoC pinctrl configuration") Cc: stable@vger.kernel.org
It looks this depends on previous commit, right?
Yes that's right, it depends on the refactoring in the previous patch. To fix the bug (which is an Serror on suspend for gs101), we need the dedicated gs101 callback so it can have the knowledge that fltcon1 doesn't always exist and it's varying offset.
and also dependent on the first patch that adds the eint_fltcon_offset :)
Peter
On 12/03/2025 12:39, Peter Griffin wrote:
Hi Krzysztof,
On Wed, 12 Mar 2025 at 11:31, Peter Griffin peter.griffin@linaro.org wrote:
Hi Krzysztof,
Thanks for the review feedback.
On Tue, 11 Mar 2025 at 19:36, Krzysztof Kozlowski krzk@kernel.org wrote:
On 07/03/2025 11:29, 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.
Fixes: 4a8be01a1a7a ("pinctrl: samsung: Add gs101 SoC pinctrl configuration") Cc: stable@vger.kernel.org
It looks this depends on previous commit, right?
Yes that's right, it depends on the refactoring in the previous patch. To fix the bug (which is an Serror on suspend for gs101), we need the dedicated gs101 callback so it can have the knowledge that fltcon1 doesn't always exist and it's varying offset.
and also dependent on the first patch that adds the eint_fltcon_offset :)
That would be fine because it's a fix as well. Ah, well, let's keep the dependency, but then I think syntax would be:
Cc: stable@vger.kernel.org # depends on the previous three patches
Best regards, Krzysztof
linux-stable-mirror@lists.linaro.org
-
Krzysztof Kozlowski -
Krzysztof Kozlowski -
Peter Griffin