Re: [PATCH v3 1/5] pinctrl: samsung: Consolidate pin-bank macros under EXYNOS9_* and pass bank_type explicitly

[Krzysztof Kozlowski <krzk@kernel.org>]

On 02/12/2025 10:36, Youngmin Nam wrote:
> Modern Exynos platforms have several near-duplicate pin-bank macro
> families (EXYNOS850_*, EXYNOS8895_*, EXYNOS7870_*), which makes
> tables verbose and harder to share across SoCs that differ only by
> bank_type (alive/off) layout.

I do not see at all how you reduced duplication, reduced verbosity or
made here anything easier to read.

Previous explicit EXYNOS850_PIN_BANK_EINTW() - so having one information
about SoC - is now containing more verbose two informations:

EXYNOS9_PIN_BANK_EINTW + exynos9_bank_type_alive,


> 
> This patch unifies them into one EXYNOS9_* macro family and makes the
> bank_type an explicit argument. The common 850-era bank types are also
> renamed to 'exynos9_bank_type_{alive,off}' to reflect their reuse on
> later Exynos 9xxx-generation parts.
> 
> Naming rationale:
> - Use of the EXYNOS9_* prefix indicates that these macros target
>   current Exynos generations sharing the same register layout and
>   EINT wiring model. Compared to SoC-specific prefixes

There is no exynos9 chip. There is also no need to rename Exynos850 to
Exynos9, because old name is correct, accurate and probably even older,
thus more suitable choice.


>   (EXYNOS850_*, EXYNOS8895_*),
>   EXYNOS9_* is clearer and more future-proof for modern parts.
> 
> Key changes:
> - Introduce:
>   - 'EXYNOS9_PIN_BANK_EINTN(bank_type, pins, reg, id)'
>   - 'EXYNOS9_PIN_BANK_EINTG(bank_type, pins, reg, id, offs)'
>   - 'EXYNOS9_PIN_BANK_EINTW(bank_type, pins, reg, id, offs)'
> - Rename:
>   - 'exynos850_bank_type_alive'-> 'exynos9_bank_type_alive'
>   - 'exynos850_bank_type_off'  -> 'exynos9_bank_type_off'
> - Convert pin-bank tables for:
>   - Exynos2200, 7870, 7885, 850, 990, 9810, 8890, 8895,
>     AutoV9, AutoV920, FSD
> - Update GS101/EXYNOSV920 helpers to reference 'exynos9_bank_type_*'
>   instead of the old exynos850 names.
> - Standardize on EXYNOS9_* macros while keeping SoC-specific
>   '*_bank_type_*' when layouts differ (e.g., 7870/8895).
> 
> No functional change intended.
> 
> Signed-off-by: Youngmin Nam <youngmin.nam@samsung.com>
> Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
> Tested-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
>  .../pinctrl/samsung/pinctrl-exynos-arm64.c    | 785 +++++++++---------
>  drivers/pinctrl/samsung/pinctrl-exynos.h      |  49 +-
>  2 files changed, 402 insertions(+), 432 deletions(-)
> 
> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> index 627dca504d7a..d11b2d4ca913 100644
> --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> @@ -53,7 +53,7 @@ static const struct samsung_pin_bank_type exynos7870_bank_type_alive = {
>   * Bank type for non-alive type. Bit fields:
>   * CON: 4, DAT: 1, PUD: 4, DRV: 4, CONPDN: 2, PUDPDN: 4
>   */
> -static const struct samsung_pin_bank_type exynos850_bank_type_off  = {
> +static const struct samsung_pin_bank_type exynos9_bank_type_off = {

This is completely pointless.

>  	.fld_width = { 4, 1, 4, 4, 2, 4, },
>  	.reg_offset = { 0x00, 0x04, 0x08, 0x0c, 0x10, 0x14, },
>  };
> @@ -62,7 +62,7 @@ static const struct samsung_pin_bank_type exynos850_bank_type_off  = {
>   * Bank type for alive type. Bit fields:
>   * CON: 4, DAT: 1, PUD: 4, DRV: 4
>   */
> -static const struct samsung_pin_bank_type exynos850_bank_type_alive = {
> +static const struct samsung_pin_bank_type exynos9_bank_type_alive = {
>  	.fld_width = { 4, 1, 4, 4, },
>  	.reg_offset = { 0x00, 0x04, 0x08, 0x0c, },
>  };
> @@ -90,11 +90,11 @@ static atomic_t exynos_shared_retention_refcnt;
>  
>  /* pin banks of exynos2200 pin-controller - ALIVE */
>  static const struct samsung_pin_bank_data exynos2200_pin_banks0[] __initconst = {
> -	EXYNOS850_PIN_BANK_EINTW(8, 0x0, "gpa0", 0x00),
> -	EXYNOS850_PIN_BANK_EINTW(8, 0x20, "gpa1", 0x04),
> -	EXYNOS850_PIN_BANK_EINTW(8, 0x40, "gpa2", 0x08),
> -	EXYNOS850_PIN_BANK_EINTW(8, 0x60, "gpa3", 0x0c),
> -	EXYNOS850_PIN_BANK_EINTW(2, 0x80, "gpa4", 0x10),
> +	EXYNOS9_PIN_BANK_EINTW(exynos9_bank_type_alive, 8, 0x0, "gpa0", 0x00),

And this is less readable.

Best regards,
Krzysztof