diff mbox series

[v5,14/20] pinctrl: samsung: Add gs101 SoC pinctrl configuration

Message ID 20231201160925.3136868-15-peter.griffin@linaro.org
State New
Headers show
Series Add minimal Tensor/GS101 SoC support and Oriole/Pixel6 board | expand

Commit Message

Peter Griffin Dec. 1, 2023, 4:09 p.m. UTC
Add support for the pin-controller found on the gs101 SoC used in
Pixel 6 phones.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 .../pinctrl/samsung/pinctrl-exynos-arm64.c    | 159 ++++++++++++++++++
 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, 198 insertions(+)

Comments

Will McVicker Dec. 1, 2023, 11:57 p.m. UTC | #1
On 12/01/2023, Peter Griffin wrote:
> Add support for the pin-controller found on the gs101 SoC used in
> Pixel 6 phones.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>

Tested-by: Will McVicker <willmcvicker@google.com>

---

I verified boot and that the pinctrl probes.

Regards,
Will

> ---
>  .../pinctrl/samsung/pinctrl-exynos-arm64.c    | 159 ++++++++++++++++++
>  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, 198 insertions(+)
> 
> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> index cb965cf93705..e1a0668ecb16 100644
> --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> @@ -796,3 +796,162 @@ 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(8, 0x0, "gpa0", 0x00, 0x00),
> +	EXYNOS9_PIN_BANK_EINTW(7, 0x20, "gpa1", 0x04, 0x08),
> +	EXYNOS9_PIN_BANK_EINTW(5, 0x40, "gpa2", 0x08, 0x10),
> +	EXYNOS9_PIN_BANK_EINTW(4, 0x60, "gpa3", 0x0c, 0x18),
> +	EXYNOS9_PIN_BANK_EINTW(4, 0x80, "gpa4", 0x10, 0x1c),
> +	EXYNOS9_PIN_BANK_EINTW(7, 0xa0, "gpa5", 0x14, 0x20),
> +	EXYNOS9_PIN_BANK_EINTW(8, 0xc0, "gpa9", 0x18, 0x28),
> +	EXYNOS9_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[] = {
> +	EXYNOS9_PIN_BANK_EINTW(8, 0x0, "gpa6", 0x00, 0x00),
> +	EXYNOS9_PIN_BANK_EINTW(4, 0x20, "gpa7", 0x04, 0x08),
> +	EXYNOS9_PIN_BANK_EINTW(8, 0x40, "gpa8", 0x08, 0x0c),
> +	EXYNOS9_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[] = {
> +	EXYNOS9_PIN_BANK_EINTG(2, 0x0, "gps0", 0x00, 0x00),
> +	EXYNOS9_PIN_BANK_EINTG(8, 0x20, "gps1", 0x04, 0x04),
> +	EXYNOS9_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[] = {
> +	EXYNOS9_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[] = {
> +	EXYNOS9_PIN_BANK_EINTG(5, 0x0, "gpp0", 0x00, 0x00),
> +	EXYNOS9_PIN_BANK_EINTG(4, 0x20, "gpp1", 0x04, 0x08),
> +	EXYNOS9_PIN_BANK_EINTG(4, 0x40, "gpp2", 0x08, 0x0c),
> +	EXYNOS9_PIN_BANK_EINTG(2, 0x60, "gpp3", 0x0c, 0x10),
> +	EXYNOS9_PIN_BANK_EINTG(4, 0x80, "gpp4", 0x10, 0x14),
> +	EXYNOS9_PIN_BANK_EINTG(2, 0xa0, "gpp5", 0x14, 0x18),
> +	EXYNOS9_PIN_BANK_EINTG(4, 0xc0, "gpp6", 0x18, 0x1c),
> +	EXYNOS9_PIN_BANK_EINTG(2, 0xe0, "gpp7", 0x1c, 0x20),
> +	EXYNOS9_PIN_BANK_EINTG(4, 0x100, "gpp8", 0x20, 0x24),
> +	EXYNOS9_PIN_BANK_EINTG(2, 0x120, "gpp9", 0x24, 0x28),
> +	EXYNOS9_PIN_BANK_EINTG(4, 0x140, "gpp10", 0x28, 0x2c),
> +	EXYNOS9_PIN_BANK_EINTG(2, 0x160, "gpp11", 0x2c, 0x30),
> +	EXYNOS9_PIN_BANK_EINTG(4, 0x180, "gpp12", 0x30, 0x34),
> +	EXYNOS9_PIN_BANK_EINTG(2, 0x1a0, "gpp13", 0x34, 0x38),
> +	EXYNOS9_PIN_BANK_EINTG(4, 0x1c0, "gpp14", 0x38, 0x3c),
> +	EXYNOS9_PIN_BANK_EINTG(2, 0x1e0, "gpp15", 0x3c, 0x40),
> +	EXYNOS9_PIN_BANK_EINTG(4, 0x200, "gpp16", 0x40, 0x44),
> +	EXYNOS9_PIN_BANK_EINTG(2, 0x220, "gpp17", 0x44, 0x48),
> +	EXYNOS9_PIN_BANK_EINTG(4, 0x240, "gpp18", 0x48, 0x4c),
> +	EXYNOS9_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[] = {
> +	EXYNOS9_PIN_BANK_EINTG(8, 0x0, "gpp20", 0x00, 0x00),
> +	EXYNOS9_PIN_BANK_EINTG(4, 0x20, "gpp21", 0x04, 0x08),
> +	EXYNOS9_PIN_BANK_EINTG(2, 0x40, "gpp22", 0x08, 0x0c),
> +	EXYNOS9_PIN_BANK_EINTG(8, 0x60, "gpp23", 0x0c, 0x10),
> +	EXYNOS9_PIN_BANK_EINTG(4, 0x80, "gpp24", 0x10, 0x18),
> +	EXYNOS9_PIN_BANK_EINTG(4, 0xa0, "gpp25", 0x14, 0x1c),
> +	EXYNOS9_PIN_BANK_EINTG(5, 0xc0, "gpp26", 0x18, 0x20),
> +	EXYNOS9_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[] = {
> +	EXYNOS9_PIN_BANK_EINTG(6, 0x0, "gph0", 0x00, 0x00),
> +	EXYNOS9_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[] = {
> +	EXYNOS9_PIN_BANK_EINTG(6, 0x0, "gph2", 0x00, 0x00),
> +	EXYNOS9_PIN_BANK_EINTG(2, 0x20, "gph3", 0x04, 0x08),
> +	EXYNOS9_PIN_BANK_EINTG(6, 0x40, "gph4", 0x08, 0x0c),
> +};
> +
> +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_wkup_init = exynos_eint_wkup_init,
> +		.suspend	= exynos_pinctrl_suspend,
> +		.resume		= exynos_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,
> +	}, {
> +		/* pin banks of gs101 pin-controller (GSACORE) */
> +		.pin_banks	= gs101_pin_gsacore,
> +		.nr_banks	= ARRAY_SIZE(gs101_pin_gsacore),
> +	}, {
> +		/* pin banks of gs101 pin-controller (GSACTRL) */
> +		.pin_banks	= gs101_pin_gsactrl,
> +		.nr_banks	= ARRAY_SIZE(gs101_pin_gsactrl),
> +	}, {
> +		/* 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 56fc11a1fe2f..75b9cf72ce73 100644
> --- a/drivers/pinctrl/samsung/pinctrl-exynos.c
> +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c
> @@ -537,6 +537,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 e2799ff1b5e9..1ffc90db079d 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(pins, reg, id, offs, fltcon_offs) \
> +	{						\
> +		.type		= &gs101_bank_type_off,	\
> +		.pctl_offset	= reg,			\
> +		.nr_pins	= pins,			\
> +		.eint_type	= EINT_TYPE_GPIO,	\
> +		.eint_offset	= offs,			\
> +		.fltcon_type    = FLT_DEFAULT,		\
> +		.fltcon_offset	= fltcon_offs,		\
> +		.name		= id			\
> +	}
> +
> +#define EXYNOS9_PIN_BANK_EINTW(pins, reg, id, offs, fltcon_offs) \
> +	{							\
> +		.type		= &gs101_bank_type_alive,	\
> +		.pctl_offset	= reg,				\
> +		.nr_pins	= pins,				\
> +		.eint_type	= EINT_TYPE_WKUP,		\
> +		.eint_offset	= offs,				\
> +		.fltcon_type    = FLT_SELECTABLE,		\
> +		.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 50c360b4753a..982a5702714c 100644
> --- a/drivers/pinctrl/samsung/pinctrl-samsung.c
> +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
> @@ -1323,6 +1323,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 5fab3885a7d7..f6856290608c 100644
> --- a/drivers/pinctrl/samsung/pinctrl-samsung.h
> +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h
> @@ -373,6 +373,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.43.0.rc2.451.g8631bc7472-goog
> 
> -- 
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
>
Sam Protsenko Dec. 2, 2023, 12:40 a.m. UTC | #2
On Fri, Dec 1, 2023 at 10:11 AM Peter Griffin <peter.griffin@linaro.org> wrote:
>
> Add support for the pin-controller found on the gs101 SoC used in
> Pixel 6 phones.
>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  .../pinctrl/samsung/pinctrl-exynos-arm64.c    | 159 ++++++++++++++++++
>  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, 198 insertions(+)
>
> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> index cb965cf93705..e1a0668ecb16 100644
> --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> @@ -796,3 +796,162 @@ 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, },
> +};

This is just the same as exynos850_bank_type_off (100% duplication).
Here is what I suggest. Now that it's obvious there is some common
platform for moder Exynos SoCs, and it's probably Exynos9, I'd suggest
next course of action (if maintainers agree):
  1. Remove this one
  2. Rename exynos850_bank_type_off to exynos9_bank_type_off
  3. Use it for both gs101 and exynos850

Does it make sense?

> +
> +/*
> + * 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, },
> +};

Ditto, it's a duplication of exynos850_bank_type_alive .

> +
> +/* pin banks of gs101 pin-controller (ALIVE) */
> +static const struct samsung_pin_bank_data gs101_pin_alive[] = {
> +       EXYNOS9_PIN_BANK_EINTW(8, 0x0, "gpa0", 0x00, 0x00),
> +       EXYNOS9_PIN_BANK_EINTW(7, 0x20, "gpa1", 0x04, 0x08),
> +       EXYNOS9_PIN_BANK_EINTW(5, 0x40, "gpa2", 0x08, 0x10),
> +       EXYNOS9_PIN_BANK_EINTW(4, 0x60, "gpa3", 0x0c, 0x18),
> +       EXYNOS9_PIN_BANK_EINTW(4, 0x80, "gpa4", 0x10, 0x1c),
> +       EXYNOS9_PIN_BANK_EINTW(7, 0xa0, "gpa5", 0x14, 0x20),
> +       EXYNOS9_PIN_BANK_EINTW(8, 0xc0, "gpa9", 0x18, 0x28),
> +       EXYNOS9_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[] = {
> +       EXYNOS9_PIN_BANK_EINTW(8, 0x0, "gpa6", 0x00, 0x00),
> +       EXYNOS9_PIN_BANK_EINTW(4, 0x20, "gpa7", 0x04, 0x08),
> +       EXYNOS9_PIN_BANK_EINTW(8, 0x40, "gpa8", 0x08, 0x0c),
> +       EXYNOS9_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[] = {
> +       EXYNOS9_PIN_BANK_EINTG(2, 0x0, "gps0", 0x00, 0x00),
> +       EXYNOS9_PIN_BANK_EINTG(8, 0x20, "gps1", 0x04, 0x04),
> +       EXYNOS9_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[] = {
> +       EXYNOS9_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[] = {
> +       EXYNOS9_PIN_BANK_EINTG(5, 0x0, "gpp0", 0x00, 0x00),
> +       EXYNOS9_PIN_BANK_EINTG(4, 0x20, "gpp1", 0x04, 0x08),
> +       EXYNOS9_PIN_BANK_EINTG(4, 0x40, "gpp2", 0x08, 0x0c),
> +       EXYNOS9_PIN_BANK_EINTG(2, 0x60, "gpp3", 0x0c, 0x10),
> +       EXYNOS9_PIN_BANK_EINTG(4, 0x80, "gpp4", 0x10, 0x14),
> +       EXYNOS9_PIN_BANK_EINTG(2, 0xa0, "gpp5", 0x14, 0x18),
> +       EXYNOS9_PIN_BANK_EINTG(4, 0xc0, "gpp6", 0x18, 0x1c),
> +       EXYNOS9_PIN_BANK_EINTG(2, 0xe0, "gpp7", 0x1c, 0x20),
> +       EXYNOS9_PIN_BANK_EINTG(4, 0x100, "gpp8", 0x20, 0x24),
> +       EXYNOS9_PIN_BANK_EINTG(2, 0x120, "gpp9", 0x24, 0x28),
> +       EXYNOS9_PIN_BANK_EINTG(4, 0x140, "gpp10", 0x28, 0x2c),
> +       EXYNOS9_PIN_BANK_EINTG(2, 0x160, "gpp11", 0x2c, 0x30),
> +       EXYNOS9_PIN_BANK_EINTG(4, 0x180, "gpp12", 0x30, 0x34),
> +       EXYNOS9_PIN_BANK_EINTG(2, 0x1a0, "gpp13", 0x34, 0x38),
> +       EXYNOS9_PIN_BANK_EINTG(4, 0x1c0, "gpp14", 0x38, 0x3c),
> +       EXYNOS9_PIN_BANK_EINTG(2, 0x1e0, "gpp15", 0x3c, 0x40),
> +       EXYNOS9_PIN_BANK_EINTG(4, 0x200, "gpp16", 0x40, 0x44),
> +       EXYNOS9_PIN_BANK_EINTG(2, 0x220, "gpp17", 0x44, 0x48),
> +       EXYNOS9_PIN_BANK_EINTG(4, 0x240, "gpp18", 0x48, 0x4c),
> +       EXYNOS9_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[] = {
> +       EXYNOS9_PIN_BANK_EINTG(8, 0x0, "gpp20", 0x00, 0x00),
> +       EXYNOS9_PIN_BANK_EINTG(4, 0x20, "gpp21", 0x04, 0x08),
> +       EXYNOS9_PIN_BANK_EINTG(2, 0x40, "gpp22", 0x08, 0x0c),
> +       EXYNOS9_PIN_BANK_EINTG(8, 0x60, "gpp23", 0x0c, 0x10),
> +       EXYNOS9_PIN_BANK_EINTG(4, 0x80, "gpp24", 0x10, 0x18),
> +       EXYNOS9_PIN_BANK_EINTG(4, 0xa0, "gpp25", 0x14, 0x1c),
> +       EXYNOS9_PIN_BANK_EINTG(5, 0xc0, "gpp26", 0x18, 0x20),
> +       EXYNOS9_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[] = {
> +       EXYNOS9_PIN_BANK_EINTG(6, 0x0, "gph0", 0x00, 0x00),
> +       EXYNOS9_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[] = {
> +       EXYNOS9_PIN_BANK_EINTG(6, 0x0, "gph2", 0x00, 0x00),
> +       EXYNOS9_PIN_BANK_EINTG(2, 0x20, "gph3", 0x04, 0x08),
> +       EXYNOS9_PIN_BANK_EINTG(6, 0x40, "gph4", 0x08, 0x0c),
> +};
> +
> +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_wkup_init = exynos_eint_wkup_init,
> +               .suspend        = exynos_pinctrl_suspend,
> +               .resume         = exynos_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,
> +       }, {
> +               /* pin banks of gs101 pin-controller (GSACORE) */
> +               .pin_banks      = gs101_pin_gsacore,
> +               .nr_banks       = ARRAY_SIZE(gs101_pin_gsacore),
> +       }, {
> +               /* pin banks of gs101 pin-controller (GSACTRL) */
> +               .pin_banks      = gs101_pin_gsactrl,
> +               .nr_banks       = ARRAY_SIZE(gs101_pin_gsactrl),
> +       }, {
> +               /* 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 56fc11a1fe2f..75b9cf72ce73 100644
> --- a/drivers/pinctrl/samsung/pinctrl-exynos.c
> +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c
> @@ -537,6 +537,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 e2799ff1b5e9..1ffc90db079d 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                    \
> +       }

Where exactly it is used? What is 'types'? Moreover, it doesn't look
very different from EXYNOS850_PIN_BANK_EINTN() -- just because I
created EXYNOS850_PIN_BANK_EINTN() exactly from this downstream macro.
I was strictly prohibited adding EXYNOS9_* stuff back at the time. But
now I guess it should be apparent we are actually dealing with Exynos9
common platform. So I suggest renaming EXYNOS850_PIN_BANK_EINTN() to
EXYNOS9_PIN_BANK_EINTN().

> +
> +#define EXYNOS9_PIN_BANK_EINTG(pins, reg, id, offs, fltcon_offs) \
> +       {                                               \
> +               .type           = &gs101_bank_type_off, \
> +               .pctl_offset    = reg,                  \
> +               .nr_pins        = pins,                 \
> +               .eint_type      = EINT_TYPE_GPIO,       \
> +               .eint_offset    = offs,                 \
> +               .fltcon_type    = FLT_DEFAULT,          \
> +               .fltcon_offset  = fltcon_offs,          \
> +               .name           = id                    \
> +       }

Ditto. Please add filter fields to EXYNOS850_PIN_BANK_EINTG() instead
of adding pretty much the same macro (it is the same, I created
EXYNOS850_PIN_BANK_EINTG() from exactly this macro). Also I suggest
renaming EXYNOS850_PIN_BANK_EINTG() to EXYNOS9_PIN_BANK_EINTG(), to
avoid confusion. If you need any help with reworking Exynos850 pinctrl
correspondingly, please let me know.

> +
> +#define EXYNOS9_PIN_BANK_EINTW(pins, reg, id, offs, fltcon_offs) \
> +       {                                                       \
> +               .type           = &gs101_bank_type_alive,       \
> +               .pctl_offset    = reg,                          \
> +               .nr_pins        = pins,                         \
> +               .eint_type      = EINT_TYPE_WKUP,               \
> +               .eint_offset    = offs,                         \
> +               .fltcon_type    = FLT_SELECTABLE,               \
> +               .fltcon_offset  = fltcon_offs,                  \
> +               .name           = id                            \
> +       }

Ditto.

> +
>  /**
>   * 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 50c360b4753a..982a5702714c 100644
> --- a/drivers/pinctrl/samsung/pinctrl-samsung.c
> +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
> @@ -1323,6 +1323,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 5fab3885a7d7..f6856290608c 100644
> --- a/drivers/pinctrl/samsung/pinctrl-samsung.h
> +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h
> @@ -373,6 +373,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.43.0.rc2.451.g8631bc7472-goog
>
Alim Akhtar Dec. 2, 2023, 1:36 a.m. UTC | #3
> -----Original Message-----
> From: Sam Protsenko <semen.protsenko@linaro.org>
> Sent: Saturday, December 2, 2023 6:10 AM
> To: Peter Griffin <peter.griffin@linaro.org>
> Cc: robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> mturquette@baylibre.com; conor+dt@kernel.org; sboyd@kernel.org;
> tomasz.figa@gmail.com; s.nawrocki@samsung.com; linus.walleij@linaro.org;
> wim@linux-watchdog.org; linux@roeck-us.net; catalin.marinas@arm.com;
> will@kernel.org; arnd@arndb.de; olof@lixom.net;
> gregkh@linuxfoundation.org; jirislaby@kernel.org;
> cw00.choi@samsung.com; alim.akhtar@samsung.com;
> tudor.ambarus@linaro.org; andre.draszik@linaro.org;
> saravanak@google.com; willmcvicker@google.com; soc@kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> samsung-soc@vger.kernel.org; linux-clk@vger.kernel.org; linux-
> gpio@vger.kernel.org; linux-watchdog@vger.kernel.org; kernel-
> team@android.com; linux-serial@vger.kernel.org
> Subject: Re: [PATCH v5 14/20] pinctrl: samsung: Add gs101 SoC pinctrl
> configuration
> 
> On Fri, Dec 1, 2023 at 10:11 AM Peter Griffin <peter.griffin@linaro.org>
> wrote:
> >
> > Add support for the pin-controller found on the gs101 SoC used in
> > Pixel 6 phones.
> >
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > ---
> >  .../pinctrl/samsung/pinctrl-exynos-arm64.c    | 159 ++++++++++++++++++
> >  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, 198 insertions(+)
> >
> > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> > b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> > index cb965cf93705..e1a0668ecb16 100644
> > --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> > +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> > @@ -796,3 +796,162 @@ 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, }, };
> 
> This is just the same as exynos850_bank_type_off (100% duplication).
> Here is what I suggest. Now that it's obvious there is some common platform
> for moder Exynos SoCs, and it's probably Exynos9, I'd suggest next course of
> action (if maintainers agree):
>   1. Remove this one
>   2. Rename exynos850_bank_type_off to exynos9_bank_type_off
>   3. Use it for both gs101 and exynos850
> 
> Does it make sense?
> 
My opinion is to reuse exynos850 for gs101 (wherever applicable), same philosophy was historically followed in this file.
That way (using exynos850 for gs101) things will be simple. 
Adding exynos9_* is not adding any benefit, rather it create confusion.

> > +
> > +/*
> > + * 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, }, };
[...]
Sam Protsenko Dec. 2, 2023, 1:58 a.m. UTC | #4
On Fri, Dec 1, 2023 at 7:37 PM Alim Akhtar <alim.akhtar@samsung.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Sam Protsenko <semen.protsenko@linaro.org>
> > Sent: Saturday, December 2, 2023 6:10 AM
> > To: Peter Griffin <peter.griffin@linaro.org>
> > Cc: robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> > mturquette@baylibre.com; conor+dt@kernel.org; sboyd@kernel.org;
> > tomasz.figa@gmail.com; s.nawrocki@samsung.com; linus.walleij@linaro.org;
> > wim@linux-watchdog.org; linux@roeck-us.net; catalin.marinas@arm.com;
> > will@kernel.org; arnd@arndb.de; olof@lixom.net;
> > gregkh@linuxfoundation.org; jirislaby@kernel.org;
> > cw00.choi@samsung.com; alim.akhtar@samsung.com;
> > tudor.ambarus@linaro.org; andre.draszik@linaro.org;
> > saravanak@google.com; willmcvicker@google.com; soc@kernel.org;
> > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> > samsung-soc@vger.kernel.org; linux-clk@vger.kernel.org; linux-
> > gpio@vger.kernel.org; linux-watchdog@vger.kernel.org; kernel-
> > team@android.com; linux-serial@vger.kernel.org
> > Subject: Re: [PATCH v5 14/20] pinctrl: samsung: Add gs101 SoC pinctrl
> > configuration
> >
> > On Fri, Dec 1, 2023 at 10:11 AM Peter Griffin <peter.griffin@linaro.org>
> > wrote:
> > >
> > > Add support for the pin-controller found on the gs101 SoC used in
> > > Pixel 6 phones.
> > >
> > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > > ---
> > >  .../pinctrl/samsung/pinctrl-exynos-arm64.c    | 159 ++++++++++++++++++
> > >  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, 198 insertions(+)
> > >
> > > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> > > b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> > > index cb965cf93705..e1a0668ecb16 100644
> > > --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> > > +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> > > @@ -796,3 +796,162 @@ 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, }, };
> >
> > This is just the same as exynos850_bank_type_off (100% duplication).
> > Here is what I suggest. Now that it's obvious there is some common platform
> > for moder Exynos SoCs, and it's probably Exynos9, I'd suggest next course of
> > action (if maintainers agree):
> >   1. Remove this one
> >   2. Rename exynos850_bank_type_off to exynos9_bank_type_off
> >   3. Use it for both gs101 and exynos850
> >
> > Does it make sense?
> >
> My opinion is to reuse exynos850 for gs101 (wherever applicable), same philosophy was historically followed in this file.
> That way (using exynos850 for gs101) things will be simple.
> Adding exynos9_* is not adding any benefit, rather it create confusion.
>

Yes. But why not also rename exynos850_* to exynos9_*? I've a feeling
that a lot of modern Exynos SoCs have the same pin bank configuration.
Wouldn't it be better to use exynos9_ prefix for all such SoCs than
exynos850_*? Because Exynos9 is a family, but Exynos850 is a
particular SoC from that family.

> > > +
> > > +/*
> > > + * 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, }, };
> [...]
>
>
Peter Griffin Dec. 5, 2023, 9:24 p.m. UTC | #5
Hi Sam,

Thanks for the review.

On Sat, 2 Dec 2023 at 00:40, Sam Protsenko <semen.protsenko@linaro.org> wrote:
>
> On Fri, Dec 1, 2023 at 10:11 AM Peter Griffin <peter.griffin@linaro.org> wrote:
> >
> > Add support for the pin-controller found on the gs101 SoC used in
> > Pixel 6 phones.
> >
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > ---
> >  .../pinctrl/samsung/pinctrl-exynos-arm64.c    | 159 ++++++++++++++++++
> >  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, 198 insertions(+)
> >
> > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> > index cb965cf93705..e1a0668ecb16 100644
> > --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> > +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> > @@ -796,3 +796,162 @@ 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, },
> > +};
>
> This is just the same as exynos850_bank_type_off (100% duplication).

Ah nice, I hadn't spotted that these structs matched exynos850.

> Here is what I suggest. Now that it's obvious there is some common
> platform for moder Exynos SoCs, and it's probably Exynos9, I'd suggest
> next course of action (if maintainers agree):
>   1. Remove this one
>   2. Rename exynos850_bank_type_off to exynos9_bank_type_off
>   3. Use it for both gs101 and exynos850
>
> Does it make sense?

Yes, sounds like a good plan to me. I just checked downstream and they
were called bank_type_6 and bank_type_7 originally.

@Krzysztof - are you OK with the proposed generic name above
(exynos9_bank_type_off?) to be used by both exynos850 and gs101 (and
also future Tensor exynos based SoCs)?

>
> > +
> > +/*
> > + * 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, },
> > +};
>
> Ditto, it's a duplication of exynos850_bank_type_alive .
>
> > +
> > +/* pin banks of gs101 pin-controller (ALIVE) */
> > +static const struct samsung_pin_bank_data gs101_pin_alive[] = {
> > +       EXYNOS9_PIN_BANK_EINTW(8, 0x0, "gpa0", 0x00, 0x00),
> > +       EXYNOS9_PIN_BANK_EINTW(7, 0x20, "gpa1", 0x04, 0x08),
> > +       EXYNOS9_PIN_BANK_EINTW(5, 0x40, "gpa2", 0x08, 0x10),
> > +       EXYNOS9_PIN_BANK_EINTW(4, 0x60, "gpa3", 0x0c, 0x18),
> > +       EXYNOS9_PIN_BANK_EINTW(4, 0x80, "gpa4", 0x10, 0x1c),
> > +       EXYNOS9_PIN_BANK_EINTW(7, 0xa0, "gpa5", 0x14, 0x20),
> > +       EXYNOS9_PIN_BANK_EINTW(8, 0xc0, "gpa9", 0x18, 0x28),
> > +       EXYNOS9_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[] = {
> > +       EXYNOS9_PIN_BANK_EINTW(8, 0x0, "gpa6", 0x00, 0x00),
> > +       EXYNOS9_PIN_BANK_EINTW(4, 0x20, "gpa7", 0x04, 0x08),
> > +       EXYNOS9_PIN_BANK_EINTW(8, 0x40, "gpa8", 0x08, 0x0c),
> > +       EXYNOS9_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[] = {
> > +       EXYNOS9_PIN_BANK_EINTG(2, 0x0, "gps0", 0x00, 0x00),
> > +       EXYNOS9_PIN_BANK_EINTG(8, 0x20, "gps1", 0x04, 0x04),
> > +       EXYNOS9_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[] = {
> > +       EXYNOS9_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[] = {
> > +       EXYNOS9_PIN_BANK_EINTG(5, 0x0, "gpp0", 0x00, 0x00),
> > +       EXYNOS9_PIN_BANK_EINTG(4, 0x20, "gpp1", 0x04, 0x08),
> > +       EXYNOS9_PIN_BANK_EINTG(4, 0x40, "gpp2", 0x08, 0x0c),
> > +       EXYNOS9_PIN_BANK_EINTG(2, 0x60, "gpp3", 0x0c, 0x10),
> > +       EXYNOS9_PIN_BANK_EINTG(4, 0x80, "gpp4", 0x10, 0x14),
> > +       EXYNOS9_PIN_BANK_EINTG(2, 0xa0, "gpp5", 0x14, 0x18),
> > +       EXYNOS9_PIN_BANK_EINTG(4, 0xc0, "gpp6", 0x18, 0x1c),
> > +       EXYNOS9_PIN_BANK_EINTG(2, 0xe0, "gpp7", 0x1c, 0x20),
> > +       EXYNOS9_PIN_BANK_EINTG(4, 0x100, "gpp8", 0x20, 0x24),
> > +       EXYNOS9_PIN_BANK_EINTG(2, 0x120, "gpp9", 0x24, 0x28),
> > +       EXYNOS9_PIN_BANK_EINTG(4, 0x140, "gpp10", 0x28, 0x2c),
> > +       EXYNOS9_PIN_BANK_EINTG(2, 0x160, "gpp11", 0x2c, 0x30),
> > +       EXYNOS9_PIN_BANK_EINTG(4, 0x180, "gpp12", 0x30, 0x34),
> > +       EXYNOS9_PIN_BANK_EINTG(2, 0x1a0, "gpp13", 0x34, 0x38),
> > +       EXYNOS9_PIN_BANK_EINTG(4, 0x1c0, "gpp14", 0x38, 0x3c),
> > +       EXYNOS9_PIN_BANK_EINTG(2, 0x1e0, "gpp15", 0x3c, 0x40),
> > +       EXYNOS9_PIN_BANK_EINTG(4, 0x200, "gpp16", 0x40, 0x44),
> > +       EXYNOS9_PIN_BANK_EINTG(2, 0x220, "gpp17", 0x44, 0x48),
> > +       EXYNOS9_PIN_BANK_EINTG(4, 0x240, "gpp18", 0x48, 0x4c),
> > +       EXYNOS9_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[] = {
> > +       EXYNOS9_PIN_BANK_EINTG(8, 0x0, "gpp20", 0x00, 0x00),
> > +       EXYNOS9_PIN_BANK_EINTG(4, 0x20, "gpp21", 0x04, 0x08),
> > +       EXYNOS9_PIN_BANK_EINTG(2, 0x40, "gpp22", 0x08, 0x0c),
> > +       EXYNOS9_PIN_BANK_EINTG(8, 0x60, "gpp23", 0x0c, 0x10),
> > +       EXYNOS9_PIN_BANK_EINTG(4, 0x80, "gpp24", 0x10, 0x18),
> > +       EXYNOS9_PIN_BANK_EINTG(4, 0xa0, "gpp25", 0x14, 0x1c),
> > +       EXYNOS9_PIN_BANK_EINTG(5, 0xc0, "gpp26", 0x18, 0x20),
> > +       EXYNOS9_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[] = {
> > +       EXYNOS9_PIN_BANK_EINTG(6, 0x0, "gph0", 0x00, 0x00),
> > +       EXYNOS9_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[] = {
> > +       EXYNOS9_PIN_BANK_EINTG(6, 0x0, "gph2", 0x00, 0x00),
> > +       EXYNOS9_PIN_BANK_EINTG(2, 0x20, "gph3", 0x04, 0x08),
> > +       EXYNOS9_PIN_BANK_EINTG(6, 0x40, "gph4", 0x08, 0x0c),
> > +};
> > +
> > +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_wkup_init = exynos_eint_wkup_init,
> > +               .suspend        = exynos_pinctrl_suspend,
> > +               .resume         = exynos_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,
> > +       }, {
> > +               /* pin banks of gs101 pin-controller (GSACORE) */
> > +               .pin_banks      = gs101_pin_gsacore,
> > +               .nr_banks       = ARRAY_SIZE(gs101_pin_gsacore),
> > +       }, {
> > +               /* pin banks of gs101 pin-controller (GSACTRL) */
> > +               .pin_banks      = gs101_pin_gsactrl,
> > +               .nr_banks       = ARRAY_SIZE(gs101_pin_gsactrl),
> > +       }, {
> > +               /* 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 56fc11a1fe2f..75b9cf72ce73 100644
> > --- a/drivers/pinctrl/samsung/pinctrl-exynos.c
> > +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c
> > @@ -537,6 +537,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 e2799ff1b5e9..1ffc90db079d 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                    \
> > +       }
>
> Where exactly it is used? What is 'types'?

EXYNOS9_PIN_BANK_EINTN macro can be dropped completely for gs101 as it
isn't used anywhere. I checked downstream code and it is declared and
not used there as well.

>Moreover, it doesn't look
> very different from EXYNOS850_PIN_BANK_EINTN() -- just because I
> created EXYNOS850_PIN_BANK_EINTN() exactly from this downstream macro.
> I was strictly prohibited adding EXYNOS9_* stuff back at the time. But
> now I guess it should be apparent we are actually dealing with Exynos9
> common platform. So I suggest renaming EXYNOS850_PIN_BANK_EINTN() to
> EXYNOS9_PIN_BANK_EINTN().
>
> > +
> > +#define EXYNOS9_PIN_BANK_EINTG(pins, reg, id, offs, fltcon_offs) \
> > +       {                                               \
> > +               .type           = &gs101_bank_type_off, \
> > +               .pctl_offset    = reg,                  \
> > +               .nr_pins        = pins,                 \
> > +               .eint_type      = EINT_TYPE_GPIO,       \
> > +               .eint_offset    = offs,                 \
> > +               .fltcon_type    = FLT_DEFAULT,          \
> > +               .fltcon_offset  = fltcon_offs,          \
> > +               .name           = id                    \
> > +       }
>
> Ditto. Please add filter fields to EXYNOS850_PIN_BANK_EINTG() instead
> of adding pretty much the same macro (it is the same, I created
> EXYNOS850_PIN_BANK_EINTG() from exactly this macro). Also I suggest
> renaming EXYNOS850_PIN_BANK_EINTG() to EXYNOS9_PIN_BANK_EINTG(), to
> avoid confusion. If you need any help with reworking Exynos850 pinctrl
> correspondingly, please let me know.

The only issue for me migrating Exynos850 to EXYNOS9_PIN_BANK_EINTG is
knowing what the fltcon_offset is. Can you provide the fltcon_offset
offsets for the relevant Exynos850 banks and help test?

Thanks,

Peter.

>
> > +
> > +#define EXYNOS9_PIN_BANK_EINTW(pins, reg, id, offs, fltcon_offs) \
> > +       {                                                       \
> > +               .type           = &gs101_bank_type_alive,       \
> > +               .pctl_offset    = reg,                          \
> > +               .nr_pins        = pins,                         \
> > +               .eint_type      = EINT_TYPE_WKUP,               \
> > +               .eint_offset    = offs,                         \
> > +               .fltcon_type    = FLT_SELECTABLE,               \
> > +               .fltcon_offset  = fltcon_offs,                  \
> > +               .name           = id                            \
> > +       }
>
> Ditto.
>
> > +
> >  /**
> >   * 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 50c360b4753a..982a5702714c 100644
> > --- a/drivers/pinctrl/samsung/pinctrl-samsung.c
> > +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
> > @@ -1323,6 +1323,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 5fab3885a7d7..f6856290608c 100644
> > --- a/drivers/pinctrl/samsung/pinctrl-samsung.h
> > +++ b/drivers/pinctrl/samsung/pinctrl-samsung.h
> > @@ -373,6 +373,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.43.0.rc2.451.g8631bc7472-goog
> >
Krzysztof Kozlowski Dec. 6, 2023, 11:38 a.m. UTC | #6
On 02/12/2023 02:36, Alim Akhtar wrote:
> 
> 
>> -----Original Message-----
>> From: Sam Protsenko <semen.protsenko@linaro.org>
>> Sent: Saturday, December 2, 2023 6:10 AM
>> To: Peter Griffin <peter.griffin@linaro.org>
>> Cc: robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
>> mturquette@baylibre.com; conor+dt@kernel.org; sboyd@kernel.org;
>> tomasz.figa@gmail.com; s.nawrocki@samsung.com; linus.walleij@linaro.org;
>> wim@linux-watchdog.org; linux@roeck-us.net; catalin.marinas@arm.com;
>> will@kernel.org; arnd@arndb.de; olof@lixom.net;
>> gregkh@linuxfoundation.org; jirislaby@kernel.org;
>> cw00.choi@samsung.com; alim.akhtar@samsung.com;
>> tudor.ambarus@linaro.org; andre.draszik@linaro.org;
>> saravanak@google.com; willmcvicker@google.com; soc@kernel.org;
>> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>> samsung-soc@vger.kernel.org; linux-clk@vger.kernel.org; linux-
>> gpio@vger.kernel.org; linux-watchdog@vger.kernel.org; kernel-
>> team@android.com; linux-serial@vger.kernel.org
>> Subject: Re: [PATCH v5 14/20] pinctrl: samsung: Add gs101 SoC pinctrl
>> configuration
>>
>> On Fri, Dec 1, 2023 at 10:11 AM Peter Griffin <peter.griffin@linaro.org>
>> wrote:
>>>
>>> Add support for the pin-controller found on the gs101 SoC used in
>>> Pixel 6 phones.
>>>
>>> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
>>> ---
>>>  .../pinctrl/samsung/pinctrl-exynos-arm64.c    | 159 ++++++++++++++++++
>>>  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, 198 insertions(+)
>>>
>>> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
>>> b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
>>> index cb965cf93705..e1a0668ecb16 100644
>>> --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
>>> +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
>>> @@ -796,3 +796,162 @@ 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, }, };
>>
>> This is just the same as exynos850_bank_type_off (100% duplication).
>> Here is what I suggest. Now that it's obvious there is some common platform
>> for moder Exynos SoCs, and it's probably Exynos9, I'd suggest next course of
>> action (if maintainers agree):
>>   1. Remove this one
>>   2. Rename exynos850_bank_type_off to exynos9_bank_type_off
>>   3. Use it for both gs101 and exynos850
>>
>> Does it make sense?
>>
> My opinion is to reuse exynos850 for gs101 (wherever applicable), same philosophy was historically followed in this file.
> That way (using exynos850 for gs101) things will be simple. 
> Adding exynos9_* is not adding any benefit, rather it create confusion.

I don't see much value in renaming exynos850 bank type to exynos9
considering:
1. We don't really know the bank types for all of Exynos9xxx SoCs,
2. Exynos7885 also uses Exynos850 bank types. Exynos7885 was much
earlier than Exynos9xxx family.

Best regards,
Krzysztof
Peter Griffin Dec. 6, 2023, 12:47 p.m. UTC | #7
Hi Krzysztof / Sam,

On Wed, 6 Dec 2023 at 11:38, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 02/12/2023 02:36, Alim Akhtar wrote:
> >
> >>
> >> On Fri, Dec 1, 2023 at 10:11 AM Peter Griffin <peter.griffin@linaro.org>
> >> wrote:
> >>>
> >>> Add support for the pin-controller found on the gs101 SoC used in
> >>> Pixel 6 phones.
> >>>
> >>> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> >>> ---
> >>>  .../pinctrl/samsung/pinctrl-exynos-arm64.c    | 159 ++++++++++++++++++
> >>>  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, 198 insertions(+)
> >>>
> >>> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> >>> b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> >>> index cb965cf93705..e1a0668ecb16 100644
> >>> --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> >>> +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> >>> @@ -796,3 +796,162 @@ 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, }, };
> >>
> >> This is just the same as exynos850_bank_type_off (100% duplication).
> >> Here is what I suggest. Now that it's obvious there is some common platform
> >> for moder Exynos SoCs, and it's probably Exynos9, I'd suggest next course of
> >> action (if maintainers agree):
> >>   1. Remove this one
> >>   2. Rename exynos850_bank_type_off to exynos9_bank_type_off
> >>   3. Use it for both gs101 and exynos850
> >>
> >> Does it make sense?
> >>
> > My opinion is to reuse exynos850 for gs101 (wherever applicable), same philosophy was historically followed in this file.
> > That way (using exynos850 for gs101) things will be simple.
> > Adding exynos9_* is not adding any benefit, rather it create confusion.
>
> I don't see much value in renaming exynos850 bank type to exynos9
> considering:
> 1. We don't really know the bank types for all of Exynos9xxx SoCs,
> 2. Exynos7885 also uses Exynos850 bank types. Exynos7885 was much
> earlier than Exynos9xxx family.

Thanks Alim and Krzysztof for your input.

Exynos7885 (Exynos 8 family) using Exynos850 bank types looks like a
mistake to me. I found some downstream code for 7885, and it doesn't
look like selecting a filter was supported downstream [1] [2]. As Sam
confirmed this hardware is present on e850 downstream, so 7885 and
e850 have different hardware at least for these banks.

As the EXYNOS850_PIN_BANK_EINTW macro is being used by Exynos850,
exynosautov9 and exynos7885 using a generic macro with gs101 doesn't
look possible (I have no way to find out these filter register
offsets, or if those platforms actually have these registers).

Therefore I propose:
1. For bank types that match exactly use exynos850 versions
2. For bank types which have fltcon_offset we add a new macro
EXYNOS9_PIN_BANK_EINTW like it exists in this series or
GS101_PIN_BANK_EINTW if people prefer that

That still leaves us in the rather unfortunate position that if
Exynos850 wants selectable filter support then it wouldn't be using
the EXYNOS850_PIN_BANK_EINTW macro. But I suggest we cross that bridge
if/when Sam decides to support selectable filters on e850. We could do
some sort of macro renaming, but what we rename it to though I have no
idea EXYNOS7885_blah or EXYNOSAUTOV9_blah.

@Chanho do you know if ExynosAutov9 supports selectable filters on alive banks?

regards,

Peter

[1] https://github.com/samsungexynos7885/android_kernel_samsung_universal7885/blob/android-9.0/drivers/pinctrl/samsung/pinctrl-exynos.c#L1696
[2] https://github.com/samsungexynos7885/android_kernel_samsung_universal7885/blob/android-9.0/drivers/pinctrl/samsung/pinctrl-exynos.h#L108
diff mbox series

Patch

diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
index cb965cf93705..e1a0668ecb16 100644
--- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
+++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
@@ -796,3 +796,162 @@  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(8, 0x0, "gpa0", 0x00, 0x00),
+	EXYNOS9_PIN_BANK_EINTW(7, 0x20, "gpa1", 0x04, 0x08),
+	EXYNOS9_PIN_BANK_EINTW(5, 0x40, "gpa2", 0x08, 0x10),
+	EXYNOS9_PIN_BANK_EINTW(4, 0x60, "gpa3", 0x0c, 0x18),
+	EXYNOS9_PIN_BANK_EINTW(4, 0x80, "gpa4", 0x10, 0x1c),
+	EXYNOS9_PIN_BANK_EINTW(7, 0xa0, "gpa5", 0x14, 0x20),
+	EXYNOS9_PIN_BANK_EINTW(8, 0xc0, "gpa9", 0x18, 0x28),
+	EXYNOS9_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[] = {
+	EXYNOS9_PIN_BANK_EINTW(8, 0x0, "gpa6", 0x00, 0x00),
+	EXYNOS9_PIN_BANK_EINTW(4, 0x20, "gpa7", 0x04, 0x08),
+	EXYNOS9_PIN_BANK_EINTW(8, 0x40, "gpa8", 0x08, 0x0c),
+	EXYNOS9_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[] = {
+	EXYNOS9_PIN_BANK_EINTG(2, 0x0, "gps0", 0x00, 0x00),
+	EXYNOS9_PIN_BANK_EINTG(8, 0x20, "gps1", 0x04, 0x04),
+	EXYNOS9_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[] = {
+	EXYNOS9_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[] = {
+	EXYNOS9_PIN_BANK_EINTG(5, 0x0, "gpp0", 0x00, 0x00),
+	EXYNOS9_PIN_BANK_EINTG(4, 0x20, "gpp1", 0x04, 0x08),
+	EXYNOS9_PIN_BANK_EINTG(4, 0x40, "gpp2", 0x08, 0x0c),
+	EXYNOS9_PIN_BANK_EINTG(2, 0x60, "gpp3", 0x0c, 0x10),
+	EXYNOS9_PIN_BANK_EINTG(4, 0x80, "gpp4", 0x10, 0x14),
+	EXYNOS9_PIN_BANK_EINTG(2, 0xa0, "gpp5", 0x14, 0x18),
+	EXYNOS9_PIN_BANK_EINTG(4, 0xc0, "gpp6", 0x18, 0x1c),
+	EXYNOS9_PIN_BANK_EINTG(2, 0xe0, "gpp7", 0x1c, 0x20),
+	EXYNOS9_PIN_BANK_EINTG(4, 0x100, "gpp8", 0x20, 0x24),
+	EXYNOS9_PIN_BANK_EINTG(2, 0x120, "gpp9", 0x24, 0x28),
+	EXYNOS9_PIN_BANK_EINTG(4, 0x140, "gpp10", 0x28, 0x2c),
+	EXYNOS9_PIN_BANK_EINTG(2, 0x160, "gpp11", 0x2c, 0x30),
+	EXYNOS9_PIN_BANK_EINTG(4, 0x180, "gpp12", 0x30, 0x34),
+	EXYNOS9_PIN_BANK_EINTG(2, 0x1a0, "gpp13", 0x34, 0x38),
+	EXYNOS9_PIN_BANK_EINTG(4, 0x1c0, "gpp14", 0x38, 0x3c),
+	EXYNOS9_PIN_BANK_EINTG(2, 0x1e0, "gpp15", 0x3c, 0x40),
+	EXYNOS9_PIN_BANK_EINTG(4, 0x200, "gpp16", 0x40, 0x44),
+	EXYNOS9_PIN_BANK_EINTG(2, 0x220, "gpp17", 0x44, 0x48),
+	EXYNOS9_PIN_BANK_EINTG(4, 0x240, "gpp18", 0x48, 0x4c),
+	EXYNOS9_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[] = {
+	EXYNOS9_PIN_BANK_EINTG(8, 0x0, "gpp20", 0x00, 0x00),
+	EXYNOS9_PIN_BANK_EINTG(4, 0x20, "gpp21", 0x04, 0x08),
+	EXYNOS9_PIN_BANK_EINTG(2, 0x40, "gpp22", 0x08, 0x0c),
+	EXYNOS9_PIN_BANK_EINTG(8, 0x60, "gpp23", 0x0c, 0x10),
+	EXYNOS9_PIN_BANK_EINTG(4, 0x80, "gpp24", 0x10, 0x18),
+	EXYNOS9_PIN_BANK_EINTG(4, 0xa0, "gpp25", 0x14, 0x1c),
+	EXYNOS9_PIN_BANK_EINTG(5, 0xc0, "gpp26", 0x18, 0x20),
+	EXYNOS9_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[] = {
+	EXYNOS9_PIN_BANK_EINTG(6, 0x0, "gph0", 0x00, 0x00),
+	EXYNOS9_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[] = {
+	EXYNOS9_PIN_BANK_EINTG(6, 0x0, "gph2", 0x00, 0x00),
+	EXYNOS9_PIN_BANK_EINTG(2, 0x20, "gph3", 0x04, 0x08),
+	EXYNOS9_PIN_BANK_EINTG(6, 0x40, "gph4", 0x08, 0x0c),
+};
+
+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_wkup_init = exynos_eint_wkup_init,
+		.suspend	= exynos_pinctrl_suspend,
+		.resume		= exynos_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,
+	}, {
+		/* pin banks of gs101 pin-controller (GSACORE) */
+		.pin_banks	= gs101_pin_gsacore,
+		.nr_banks	= ARRAY_SIZE(gs101_pin_gsacore),
+	}, {
+		/* pin banks of gs101 pin-controller (GSACTRL) */
+		.pin_banks	= gs101_pin_gsactrl,
+		.nr_banks	= ARRAY_SIZE(gs101_pin_gsactrl),
+	}, {
+		/* 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 56fc11a1fe2f..75b9cf72ce73 100644
--- a/drivers/pinctrl/samsung/pinctrl-exynos.c
+++ b/drivers/pinctrl/samsung/pinctrl-exynos.c
@@ -537,6 +537,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 e2799ff1b5e9..1ffc90db079d 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(pins, reg, id, offs, fltcon_offs) \
+	{						\
+		.type		= &gs101_bank_type_off,	\
+		.pctl_offset	= reg,			\
+		.nr_pins	= pins,			\
+		.eint_type	= EINT_TYPE_GPIO,	\
+		.eint_offset	= offs,			\
+		.fltcon_type    = FLT_DEFAULT,		\
+		.fltcon_offset	= fltcon_offs,		\
+		.name		= id			\
+	}
+
+#define EXYNOS9_PIN_BANK_EINTW(pins, reg, id, offs, fltcon_offs) \
+	{							\
+		.type		= &gs101_bank_type_alive,	\
+		.pctl_offset	= reg,				\
+		.nr_pins	= pins,				\
+		.eint_type	= EINT_TYPE_WKUP,		\
+		.eint_offset	= offs,				\
+		.fltcon_type    = FLT_SELECTABLE,		\
+		.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 50c360b4753a..982a5702714c 100644
--- a/drivers/pinctrl/samsung/pinctrl-samsung.c
+++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
@@ -1323,6 +1323,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 5fab3885a7d7..f6856290608c 100644
--- a/drivers/pinctrl/samsung/pinctrl-samsung.h
+++ b/drivers/pinctrl/samsung/pinctrl-samsung.h
@@ -373,6 +373,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;