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@xxxxxxxxxx>
> Fixes: 4a8be01a1a7a ("pinctrl: samsung: Add gs101 SoC pinctrl configuration")
> Cc: stable@xxxxxxxxxxxxxxx
> ---
> 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);
>
