Re: [PATCH v3 14/20] pinctrl: samsung: Add gs101 SoC pinctrl configuration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 11, 2023 at 1:49 PM Peter Griffin <peter.griffin@xxxxxxxxxx> wrote:
>
> Add support for the pin-controller found on the gs101 SoC used in
> Pixel 6 phones.
>
> Signed-off-by: Peter Griffin <peter.griffin@xxxxxxxxxx>
> ---
>  .../pinctrl/samsung/pinctrl-exynos-arm64.c    | 163 ++++++++++++++++++
>  drivers/pinctrl/samsung/pinctrl-exynos.c      |   2 +
>  drivers/pinctrl/samsung/pinctrl-exynos.h      |  34 ++++
>  drivers/pinctrl/samsung/pinctrl-samsung.c     |   2 +
>  drivers/pinctrl/samsung/pinctrl-samsung.h     |   1 +
>  5 files changed, 202 insertions(+)
>
> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> index cb965cf93705..db47001d1b35 100644
> --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> @@ -796,3 +796,166 @@ const struct samsung_pinctrl_of_match_data fsd_of_data __initconst = {
>         .ctrl           = fsd_pin_ctrl,
>         .num_ctrl       = ARRAY_SIZE(fsd_pin_ctrl),
>  };
> +
> +/*
> + * bank type for non-alive type
> + * (CON bit field: 4, DAT bit field: 1, PUD bit field: 4, DRV bit field: 4)
> + * (CONPDN bit field: 2, PUDPDN bit field: 4)
> + */
> +static struct samsung_pin_bank_type gs101_bank_type_off  = {
> +       .fld_width = { 4, 1, 4, 4, 2, 4, },
> +       .reg_offset = { 0x00, 0x04, 0x08, 0x0c, 0x10, 0x14, },
> +};
> +
> +/*
> + * bank type for alive type
> + * (CON bit field: 4, DAT bit field: 1, PUD bit field: 4, DRV bit field: 4)
> + */
> +static const struct samsung_pin_bank_type gs101_bank_type_alive = {
> +       .fld_width = { 4, 1, 4, 4, },
> +       .reg_offset = { 0x00, 0x04, 0x08, 0x0c, },
> +};
> +
> +/* pin banks of gs101 pin-controller (ALIVE) */
> +static const struct samsung_pin_bank_data gs101_pin_alive[] = {
> +       EXYNOS9_PIN_BANK_EINTW(gs101_bank_type_alive, 8, 0x0, "gpa0", 0x00, 0x00, FLT_SELECTABLE),

Here and further: please keep 80 characters per line when possible.

> +       EXYNOS9_PIN_BANK_EINTW(gs101_bank_type_alive, 7, 0x20, "gpa1", 0x04, 0x08, FLT_SELECTABLE),
> +       EXYNOS9_PIN_BANK_EINTW(gs101_bank_type_alive, 5, 0x40, "gpa2", 0x08, 0x10, FLT_SELECTABLE),
> +       EXYNOS9_PIN_BANK_EINTW(gs101_bank_type_alive, 4, 0x60, "gpa3", 0x0c, 0x18, FLT_SELECTABLE),
> +       EXYNOS9_PIN_BANK_EINTW(gs101_bank_type_alive, 4, 0x80, "gpa4", 0x10, 0x1c, FLT_SELECTABLE),
> +       EXYNOS9_PIN_BANK_EINTW(gs101_bank_type_alive, 7, 0xa0, "gpa5", 0x14, 0x20, FLT_SELECTABLE),
> +       EXYNOS9_PIN_BANK_EINTW(gs101_bank_type_alive, 8, 0xc0, "gpa9", 0x18, 0x28, FLT_SELECTABLE),
> +       EXYNOS9_PIN_BANK_EINTW(gs101_bank_type_alive, 2, 0xe0, "gpa10", 0x1c, 0x30, FLT_SELECTABLE),
> +};
> +
> +/* pin banks of gs101 pin-controller (FAR_ALIVE) */
> +static const struct samsung_pin_bank_data gs101_pin_far_alive[] = {
> +       EXYNOS9_PIN_BANK_EINTW(gs101_bank_type_alive, 8, 0x0, "gpa6", 0x00, 0x00, FLT_SELECTABLE),
> +       EXYNOS9_PIN_BANK_EINTW(gs101_bank_type_alive, 4, 0x20, "gpa7", 0x04, 0x08, FLT_SELECTABLE),
> +       EXYNOS9_PIN_BANK_EINTW(gs101_bank_type_alive, 8, 0x40, "gpa8", 0x08, 0x0c, FLT_SELECTABLE),
> +       EXYNOS9_PIN_BANK_EINTW(gs101_bank_type_alive, 2, 0x60, "gpa11", 0x0c, 0x14, FLT_SELECTABLE),
> +};
> +
> +/* pin banks of gs101 pin-controller (GSACORE) */
> +static const struct samsung_pin_bank_data gs101_pin_gsacore[] = {
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 2, 0x0, "gps0", 0x00, 0x00, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 8, 0x20, "gps1", 0x04, 0x04, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 3, 0x40, "gps2", 0x08, 0x0c, FLT_DEFAULT),
> +};
> +
> +/* pin banks of gs101 pin-controller (GSACTRL) */
> +static const struct samsung_pin_bank_data gs101_pin_gsactrl[] = {
> +       EXYNOS9_PIN_BANK_EINTW(gs101_bank_type_alive, 6, 0x0, "gps3", 0x00, 0x00, FLT_DEFAULT),
> +};
> +
> +/* pin banks of gs101 pin-controller (PERIC0) */
> +static const struct samsung_pin_bank_data gs101_pin_peric0[] = {
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 5, 0x0, "gpp0", 0x00, 0x00, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 4, 0x20, "gpp1", 0x04, 0x08, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 4, 0x40, "gpp2", 0x08, 0x0c, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 2, 0x60, "gpp3", 0x0c, 0x10, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 4, 0x80, "gpp4", 0x10, 0x14, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 2, 0xa0, "gpp5", 0x14, 0x18, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 4, 0xc0, "gpp6", 0x18, 0x1c, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 2, 0xe0, "gpp7", 0x1c, 0x20, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 4, 0x100, "gpp8", 0x20, 0x24, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 2, 0x120, "gpp9", 0x24, 0x28, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 4, 0x140, "gpp10", 0x28, 0x2c, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 2, 0x160, "gpp11", 0x2c, 0x30, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 4, 0x180, "gpp12", 0x30, 0x34, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 2, 0x1a0, "gpp13", 0x34, 0x38, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 4, 0x1c0, "gpp14", 0x38, 0x3c, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 2, 0x1e0, "gpp15", 0x3c, 0x40, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 4, 0x200, "gpp16", 0x40, 0x44, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 2, 0x220, "gpp17", 0x44, 0x48, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 4, 0x240, "gpp18", 0x48, 0x4c, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 4, 0x260, "gpp19", 0x4c, 0x50, FLT_DEFAULT),
> +};
> +
> +/* pin banks of gs101 pin-controller (PERIC1) */
> +static const struct samsung_pin_bank_data gs101_pin_peric1[] = {
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 8, 0x0, "gpp20", 0x00, 0x00, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 4, 0x20, "gpp21", 0x04, 0x08, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 2, 0x40, "gpp22", 0x08, 0x0c, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 8, 0x60, "gpp23", 0x0c, 0x10, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 4, 0x80, "gpp24", 0x10, 0x18, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 4, 0xa0, "gpp25", 0x14, 0x1c, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 5, 0xc0, "gpp26", 0x18, 0x20, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 4, 0xe0, "gpp27", 0x1c, 0x28, FLT_DEFAULT),
> +};
> +
> +/* pin banks of gs101 pin-controller (HSI1) */
> +static const struct samsung_pin_bank_data gs101_pin_hsi1[] = {
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 6, 0x0, "gph0", 0x00, 0x00, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 7, 0x20, "gph1", 0x04, 0x08, FLT_DEFAULT),
> +};
> +
> +/* pin banks of gs101 pin-controller (HSI2) */
> +static const struct samsung_pin_bank_data gs101_pin_hsi2[] = {
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 6, 0x0, "gph2", 0x00, 0x00, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 2, 0x20, "gph3", 0x04, 0x08, FLT_DEFAULT),
> +       EXYNOS9_PIN_BANK_EINTG(gs101_bank_type_off, 6, 0x40, "gph4", 0x08, 0x0c, FLT_DEFAULT),
> +};
> +
> +static const struct samsung_pin_ctrl gs101_pin_ctrl[] __initconst = {
> +       {
> +               /* pin banks of gs101 pin-controller (ALIVE) */
> +               .pin_banks      = gs101_pin_alive,
> +               .nr_banks       = ARRAY_SIZE(gs101_pin_alive),
> +               .eint_gpio_init = exynos_eint_gpio_init,
> +               .eint_wkup_init = exynos_eint_wkup_init,

Is it ok to have both .eint_gpio_init and .eint_wkup_init set here and
further? I remember doing something like that for Exynos850 before,
only to realize further if was a mistake. Please check commit
96f79935015c ("pinctrl: samsung: Remove EINT handler for Exynos850
ALIVE and CMGP gpios"). Maybe it's ok in your case.

> +               .suspend        = exynos_pinctrl_suspend,
> +               .resume         = exynos_pinctrl_resume,

Did you manage to actually test those suspend/resume callbacks
somehow? If so, can you please share the procedure? I guess I had some
Power Domains and clock related problems on Exynos850 when I tried
that before, so just curious.

> +       }, {
> +               /* pin banks of gs101 pin-controller (FAR_ALIVE) */
> +               .pin_banks      = gs101_pin_far_alive,
> +               .nr_banks       = ARRAY_SIZE(gs101_pin_far_alive),
> +               .eint_gpio_init = exynos_eint_gpio_init,
> +               .eint_wkup_init = exynos_eint_wkup_init,
> +               .suspend        = exynos_pinctrl_suspend,
> +               .resume         = exynos_pinctrl_resume,
> +       }, {
> +               /* pin banks of gs101 pin-controller (GSACORE) */
> +               .pin_banks      = gs101_pin_gsacore,
> +               .nr_banks       = ARRAY_SIZE(gs101_pin_gsacore),
> +               .eint_gpio_init = exynos_eint_gpio_init,
> +       }, {
> +               /* pin banks of gs101 pin-controller (GSACTRL) */
> +               .pin_banks      = gs101_pin_gsactrl,
> +               .nr_banks       = ARRAY_SIZE(gs101_pin_gsactrl),
> +               .eint_gpio_init = exynos_eint_gpio_init,
> +       }, {
> +               /* pin banks of gs101 pin-controller (PERIC0) */
> +               .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,
> +       }, {
> +               /* 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,
> +       }, {
> +               /* 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,
> +       }, {
> +               /* 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,
> +       },
> +};
> +
> +const struct samsung_pinctrl_of_match_data gs101_of_data __initconst = {
> +       .ctrl           = gs101_pin_ctrl,
> +       .num_ctrl       = ARRAY_SIZE(gs101_pin_ctrl),
> +};
> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c
> index 800831aa8357..014f0c37f97f 100644
> --- a/drivers/pinctrl/samsung/pinctrl-exynos.c
> +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c
> @@ -533,6 +533,8 @@ static const struct of_device_id exynos_wkup_irq_ids[] = {
>                         .data = &exynos7_wkup_irq_chip },
>         { .compatible = "samsung,exynosautov9-wakeup-eint",
>                         .data = &exynos7_wkup_irq_chip },
> +       { .compatible = "google,gs101-wakeup-eint",
> +                       .data = &exynos7_wkup_irq_chip },
>         { }
>  };
>
> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h b/drivers/pinctrl/samsung/pinctrl-exynos.h
> index 63b2426ad5d6..0dd013654bd2 100644
> --- a/drivers/pinctrl/samsung/pinctrl-exynos.h
> +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h
> @@ -147,6 +147,40 @@
>                 .name           = id                            \
>         }
>
> +#define EXYNOS9_PIN_BANK_EINTN(types, pins, reg, id)   \
> +       {                                               \
> +               .type           = &types,               \
> +               .pctl_offset    = reg,                  \
> +               .nr_pins        = pins,                 \
> +               .eint_type      = EINT_TYPE_NONE,       \
> +               .fltcon_type    = FLT_DEFAULT           \
> +               .name           = id                    \
> +       }
> +
> +#define EXYNOS9_PIN_BANK_EINTG(types, pins, reg, id, offs, fltcon_offs, fltcontype) \
> +       {                                               \
> +               .type           = &types,               \
> +               .pctl_offset    = reg,                  \
> +               .nr_pins        = pins,                 \
> +               .eint_type      = EINT_TYPE_GPIO,       \
> +               .eint_offset    = offs,                 \
> +               .fltcon_type    = fltcontype,           \
> +               .fltcon_offset  = fltcon_offs,          \
> +               .name           = id                    \
> +       }
> +
> +#define EXYNOS9_PIN_BANK_EINTW(types, pins, reg, id, offs, fltcon_offs, fltcontype) \
> +       {                                               \
> +               .type           = &types,               \
> +               .pctl_offset    = reg,                  \
> +               .nr_pins        = pins,                 \
> +               .eint_type      = EINT_TYPE_WKUP,       \
> +               .eint_offset    = offs,                 \
> +               .fltcon_type    = fltcontype,           \
> +               .fltcon_offset  = fltcon_offs,          \
> +               .name           = id                    \
> +       }
> +

Looks to me that instead of adding new macros the already existing
EXYNOS850_PIN_BANK_* should be extended and re-used. Because those
pinctrl IP-cores on all modern Exynos chips look very similar, even if
you compare the downstream code. If EXYNOS850 prefix looks confusing,
maybe it can be renamed to EXYNOS9 or something like that. Those
filter parameters are also present in Exynos850 downstream kernel
code. So I just feel like the proper way to add that feature would be
to add that also for all modern ARM64 Exynos variants while at it.

>  /**
>   * 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 449f8109d8b5..12176f98440d 100644
> --- a/drivers/pinctrl/samsung/pinctrl-samsung.c
> +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
> @@ -1321,6 +1321,8 @@ static const struct of_device_id samsung_pinctrl_dt_match[] = {
>                 .data = &exynosautov9_of_data },
>         { .compatible = "tesla,fsd-pinctrl",
>                 .data = &fsd_of_data },
> +       { .compatible = "google,gs101-pinctrl",
> +               .data = &gs101_of_data },
>  #endif
>  #ifdef CONFIG_PINCTRL_S3C64XX
>         { .compatible = "samsung,s3c64xx-pinctrl",
> diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h b/drivers/pinctrl/samsung/pinctrl-samsung.h
> index de2ca8e8b378..e62e909fb10d 100644
> --- a/drivers/pinctrl/samsung/pinctrl-samsung.h
> +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h
> @@ -374,6 +374,7 @@ extern const struct samsung_pinctrl_of_match_data exynos7885_of_data;
>  extern const struct samsung_pinctrl_of_match_data exynos850_of_data;
>  extern const struct samsung_pinctrl_of_match_data exynosautov9_of_data;
>  extern const struct samsung_pinctrl_of_match_data fsd_of_data;
> +extern const struct samsung_pinctrl_of_match_data gs101_of_data;
>  extern const struct samsung_pinctrl_of_match_data s3c64xx_of_data;
>  extern const struct samsung_pinctrl_of_match_data s3c2412_of_data;
>  extern const struct samsung_pinctrl_of_match_data s3c2416_of_data;
> --
> 2.42.0.655.g421f12c284-goog
>




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux