Re: [PATCH 11/23] pinctrl: samsung: add FSD SoC specific data

[Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>]

On 13/01/2022 13:11, Alim Akhtar wrote:
> This patch adds Tesla FSD SoC specific data to enable pinctrl.
> FSD SoC has similar pinctrl controller as found in the most
> samsung/exynos SoCs.
> 
> Cc: linux-fsd@tesla.com
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
>  .../pinctrl/samsung/pinctrl-exynos-arm64.c    | 71 +++++++++++++++++++
>  drivers/pinctrl/samsung/pinctrl-samsung.c     |  2 +
>  drivers/pinctrl/samsung/pinctrl-samsung.h     |  1 +
>  3 files changed, 74 insertions(+)
> 
> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> index 6b77fd24571e..b9175b4911ac 100644
> --- a/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> +++ b/drivers/pinctrl/samsung/pinctrl-exynos-arm64.c
> @@ -646,3 +646,74 @@ const struct samsung_pinctrl_of_match_data exynosautov9_of_data __initconst = {
>  	.ctrl		= exynosautov9_pin_ctrl,
>  	.num_ctrl	= ARRAY_SIZE(exynosautov9_pin_ctrl),
>  };
> +
> +/*
> + * Pinctrl driver data for Tesla FSD SoC. FSD SoC includes three
> + * gpio/pin-mux/pinconfig controllers.
> + */
> +
> +/* pin banks of FSD pin-controller 0 (FSYS) */
> +static const struct samsung_pin_bank_data fsd_pin_banks0[] __initconst = {
> +	EXYNOS850_PIN_BANK_EINTG(7, 0x00, "gpf0", 0x00),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x20, "gpf1", 0x04),
> +	EXYNOS850_PIN_BANK_EINTG(3, 0x40, "gpf6", 0x08),
> +	EXYNOS850_PIN_BANK_EINTG(2, 0x60, "gpf4", 0x0c),
> +	EXYNOS850_PIN_BANK_EINTG(6, 0x80, "gpf5", 0x10),
> +};
> +
> +/* pin banks of FSD pin-controller 1 (PERIC) */
> +static const struct samsung_pin_bank_data fsd_pin_banks1[] __initconst = {
> +	EXYNOS850_PIN_BANK_EINTG(4, 0x000, "gpc8", 0x00),
> +	EXYNOS850_PIN_BANK_EINTG(7, 0x020, "gpf2", 0x04),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x040, "gpf3", 0x08),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x060, "gpd0", 0x0c),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x080, "gpb0", 0x10),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x0a0, "gpb1", 0x14),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x0c0, "gpb4", 0x18),
> +	EXYNOS850_PIN_BANK_EINTG(4, 0x0e0, "gpb5", 0x1c),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x100, "gpb6", 0x20),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x120, "gpb7", 0x24),
> +	EXYNOS850_PIN_BANK_EINTG(5, 0x140, "gpd1", 0x28),
> +	EXYNOS850_PIN_BANK_EINTG(5, 0x160, "gpd2", 0x2c),
> +	EXYNOS850_PIN_BANK_EINTG(7, 0x180, "gpd3", 0x30),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x1a0, "gpg0", 0x34),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x1c0, "gpg1", 0x38),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x1e0, "gpg2", 0x3c),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x200, "gpg3", 0x40),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x220, "gpg4", 0x44),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x240, "gpg5", 0x48),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x260, "gpg6", 0x4c),
> +	EXYNOS850_PIN_BANK_EINTG(8, 0x280, "gpg7", 0x50),
> +};
> +
> +/* pin banks of FSD pin-controller 2 (PMU) */
> +static const struct samsung_pin_bank_data fsd_pin_banks2[] __initconst = {
> +	EXYNOS850_PIN_BANK_EINTN(3, 0x00, "gpq0"),
> +};
> +
> +const struct samsung_pin_ctrl fsd_pin_ctrl[] __initconst = {
> +	{
> +		/* pin-controller instance 0 FSYS0 data */
> +		.pin_banks	= fsd_pin_banks0,
> +		.nr_banks	= ARRAY_SIZE(fsd_pin_banks0),

No wake-up interrupts (eint_wkup_init)? It's fine not to have them but
just looks incomplete.

In general looks ok, except discussion about compatibles.

> +		.eint_gpio_init = exynos_eint_gpio_init,
> +		.suspend	= exynos_pinctrl_suspend,
> +		.resume		= exynos_pinctrl_resume,
Best regards,
Krzysztof