Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 11 Nov 2022 17:01:58 +0100
From:   Thierry Reding <thierry.reding@gmail.com>
To:     Dipen Patel <dipenp@nvidia.com>
Cc:     jonathanh@nvidia.com, linux-kernel@vger.kernel.org,
        linux-tegra@vger.kernel.org, linux-gpio@vger.kernel.org,
        linus.walleij@linaro.org, devicetree@vger.kernel.org,
        linux-doc@vger.kernel.org, robh+dt@kernel.org
Subject: Re: [PATCH 2/7] hte: Add Tegra234 provider
Message-ID: <Y25x9gc4tpF1f022@orome>
References: <20221103174523.29592-1-dipenp@nvidia.com>
 <20221103174523.29592-3-dipenp@nvidia.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="dyfI+VVQf83HGbpw"
Content-Disposition: inline
In-Reply-To: <20221103174523.29592-3-dipenp@nvidia.com>
User-Agent: Mutt/2.2.8 (2022-11-05)
Precedence: bulk


--dyfI+VVQf83HGbpw
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Nov 03, 2022 at 10:45:18AM -0700, Dipen Patel wrote:
> The Tegra234 AON GPIO instance and LIC IRQ support HTE. For the GPIO
> HTE support, it requires to add mapping between GPIO and HTE framework.
>=20
> Signed-off-by: Dipen Patel <dipenp@nvidia.com>
> ---
>  drivers/hte/hte-tegra194-test.c |   2 +-
>  drivers/hte/hte-tegra194.c      | 124 ++++++++++++++++++++++++++++++--
>  2 files changed, 121 insertions(+), 5 deletions(-)
>=20
> diff --git a/drivers/hte/hte-tegra194-test.c b/drivers/hte/hte-tegra194-t=
est.c
> index 5d776a185bd6..d79c28a80517 100644
> --- a/drivers/hte/hte-tegra194-test.c
> +++ b/drivers/hte/hte-tegra194-test.c
> @@ -16,7 +16,7 @@
>  #include <linux/hte.h>
> =20
>  /*
> - * This sample HTE GPIO test driver demonstrates HTE API usage by enabli=
ng
> + * This sample HTE test driver demonstrates HTE API usage by enabling
>   * hardware timestamp on gpio_in and specified LIC IRQ lines.
>   *
>   * Note: gpio_out and gpio_in need to be shorted externally in order for=
 this
> diff --git a/drivers/hte/hte-tegra194.c b/drivers/hte/hte-tegra194.c
> index 49a27af22742..5d1f947db0f6 100644
> --- a/drivers/hte/hte-tegra194.c
> +++ b/drivers/hte/hte-tegra194.c
> @@ -62,6 +62,10 @@
>  #define NV_AON_HTE_SLICE2_IRQ_GPIO_25	25
>  #define NV_AON_HTE_SLICE2_IRQ_GPIO_26	26
>  #define NV_AON_HTE_SLICE2_IRQ_GPIO_27	27
> +#define NV_AON_HTE_SLICE2_IRQ_GPIO_28	28
> +#define NV_AON_HTE_SLICE2_IRQ_GPIO_29	29
> +#define NV_AON_HTE_SLICE2_IRQ_GPIO_30	30
> +#define NV_AON_HTE_SLICE2_IRQ_GPIO_31	31
> =20
>  #define HTE_TECTRL		0x0
>  #define HTE_TETSCH		0x4
> @@ -220,7 +224,100 @@ static const struct tegra_hte_line_mapped tegra194_=
aon_gpio_sec_map[] =3D {
>  	[39] =3D {NV_AON_SLICE_INVALID, 0},
>  };
> =20
> -static const struct tegra_hte_data aon_hte =3D {
> +static const struct tegra_hte_line_mapped tegra234_aon_gpio_map[] =3D {
> +	/* gpio, slice, bit_index */
> +	/* AA port */
> +	[0]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_11},
> +	[1]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_10},
> +	[2]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_9},
> +	[3]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_8},
> +	[4]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_7},
> +	[5]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_6},
> +	[6]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_5},
> +	[7]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_4},
> +	/* BB port */
> +	[8]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_3},
> +	[9]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_2},
> +	[10] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_1},
> +	[11] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_0},
> +	/* CC port */
> +	[12] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_22},
> +	[13] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_21},
> +	[14] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_20},
> +	[15] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_19},
> +	[16] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_18},
> +	[17] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_17},
> +	[18] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_16},
> +	[19] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_15},
> +	/* DD port */
> +	[20] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_14},
> +	[21] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_13},
> +	[22] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_12},
> +	/* EE port */
> +	[23] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_31},
> +	[24] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_30},
> +	[25] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_29},
> +	[26] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_28},
> +	[27] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_27},
> +	[28] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_26},
> +	[29] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_25},
> +	[30] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_24},
> +	/* GG port */
> +	[31] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_23},
> +};
> +
> +static const struct tegra_hte_line_mapped tegra234_aon_gpio_sec_map[] =
=3D {
> +	/* gpio, slice, bit_index */
> +	/* AA port */
> +	[0]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_11},
> +	[1]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_10},
> +	[2]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_9},
> +	[3]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_8},
> +	[4]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_7},
> +	[5]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_6},
> +	[6]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_5},
> +	[7]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_4},
> +	/* BB port */
> +	[8]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_3},
> +	[9]  =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_2},
> +	[10] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_1},
> +	[11] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_0},
> +	[12] =3D {NV_AON_SLICE_INVALID, 0},
> +	[13] =3D {NV_AON_SLICE_INVALID, 0},
> +	[14] =3D {NV_AON_SLICE_INVALID, 0},
> +	[15] =3D {NV_AON_SLICE_INVALID, 0},
> +	/* CC port */
> +	[16] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_22},
> +	[17] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_21},
> +	[18] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_20},
> +	[19] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_19},
> +	[20] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_18},
> +	[21] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_17},
> +	[22] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_16},
> +	[23] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_15},
> +	/* DD port */
> +	[24] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_14},
> +	[25] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_13},
> +	[26] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_12},
> +	[27] =3D {NV_AON_SLICE_INVALID, 0},
> +	[28] =3D {NV_AON_SLICE_INVALID, 0},
> +	[29] =3D {NV_AON_SLICE_INVALID, 0},
> +	[30] =3D {NV_AON_SLICE_INVALID, 0},
> +	[31] =3D {NV_AON_SLICE_INVALID, 0},
> +	/* EE port */
> +	[32] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_31},
> +	[33] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_30},
> +	[34] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_29},
> +	[35] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_28},
> +	[36] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_27},
> +	[37] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_26},
> +	[38] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_25},
> +	[39] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_24},
> +	/* GG port */
> +	[40] =3D {2, NV_AON_HTE_SLICE2_IRQ_GPIO_23},
> +};
> +
> +static const struct tegra_hte_data t194_aon_hte =3D {
>  	.map_sz =3D ARRAY_SIZE(tegra194_aon_gpio_map),
>  	.map =3D tegra194_aon_gpio_map,
>  	.sec_map_sz =3D ARRAY_SIZE(tegra194_aon_gpio_sec_map),
> @@ -228,6 +325,14 @@ static const struct tegra_hte_data aon_hte =3D {
>  	.type =3D HTE_TEGRA_TYPE_GPIO,
>  };
> =20
> +static const struct tegra_hte_data t234_aon_hte =3D {
> +	.map_sz =3D ARRAY_SIZE(tegra234_aon_gpio_map),
> +	.map =3D tegra234_aon_gpio_map,
> +	.sec_map_sz =3D ARRAY_SIZE(tegra234_aon_gpio_sec_map),
> +	.sec_map =3D tegra234_aon_gpio_sec_map,
> +	.type =3D HTE_TEGRA_TYPE_GPIO,
> +};
> +
>  static const struct tegra_hte_data lic_hte =3D {
>  	.map_sz =3D 0,
>  	.map =3D NULL,
> @@ -535,7 +640,9 @@ static bool tegra_hte_match_from_linedata(const struc=
t hte_chip *chip,
> =20
>  static const struct of_device_id tegra_hte_of_match[] =3D {
>  	{ .compatible =3D "nvidia,tegra194-gte-lic", .data =3D &lic_hte},
> -	{ .compatible =3D "nvidia,tegra194-gte-aon", .data =3D &aon_hte},
> +	{ .compatible =3D "nvidia,tegra194-gte-aon", .data =3D &t194_aon_hte},
> +	{ .compatible =3D "nvidia,tegra234-gte-lic", .data =3D &lic_hte},
> +	{ .compatible =3D "nvidia,tegra234-gte-aon", .data =3D &t234_aon_hte},
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, tegra_hte_of_match);
> @@ -635,8 +742,17 @@ static int tegra_hte_probe(struct platform_device *p=
dev)
> =20
>  		gc->match_from_linedata =3D tegra_hte_match_from_linedata;
> =20
> -		hte_dev->c =3D gpiochip_find("tegra194-gpio-aon",
> -					   tegra_get_gpiochip_from_name);
> +		if (of_device_is_compatible(dev->of_node,
> +					    "nvidia,tegra194-gte-aon"))
> +			hte_dev->c =3D gpiochip_find("tegra194-gpio-aon",
> +						tegra_get_gpiochip_from_name);
> +		else if (of_device_is_compatible(dev->of_node,
> +						 "nvidia,tegra234-gte-aon"))
> +			hte_dev->c =3D gpiochip_find("tegra234-gpio-aon",
> +						tegra_get_gpiochip_from_name);
> +		else
> +			return -ENODEV;

I'm wondering: instead of doing this cumbersome lookup, perhaps it would
be easier to create a direct link to the right GPIO controller with a
phandle?

Thierry

--dyfI+VVQf83HGbpw
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAmNucfYACgkQ3SOs138+
s6HrSQ/+MdLNVwtKRzyACdsByavI0GX3zfBSNTl+UKVriDgHS9qLtb3hARiAzaCp
XI68gQKZTJZ5VvKomZWLNwO4GGglOnmd7efHjYSA3ddEGulP/6jRAd3zoLJclG+6
cmd/eAsiXQELX35Zs2pkVWZb9Yor13ZldF+29R9QxtzC49v7rU0uJMctEMkuq0/R
p4oducDhbsAIgPT+PqPMfUIzYuOvTdHpokNZn9J0nhZzYPqOiuztDTIVjtv0MI9c
7kkP1Ar7oyoMshMwOQz4rmTMTNn/nQFRcpS9ur6iRCtNbxrLxwDiBMqYLARfAPUM
j4O0vKR6dlMuhJSTiSGbnfY3paWLaS9y1/InCkNAJ0wUsry+CbyNts0qv+A7aqec
djbmkPGYKFdjqdwvtuYR41My+M0wBN/AzEuCNIgeWL+IP6HGu7fDz0q+DFdDLfAn
sNisK9k9SgKghX1IFw00nQLF6CEbMxz5a2ovJCB1Zce0QrbBAL94M92K3JXFTFDF
75rBD8Ylw3RKFyZRqnCbwB6s4f9i0/ukK/mg3qmRwkCvaFvRTHJ36nTQZEksoFl3
iOwUuXdkTFSJcvVQRD8HVuoSTj0Xbhpx0Byk+S5exP1UsD3L6nKFK9/tVKDth1Tn
pmF9BFS+tGyqWJdPTf9kVdSdOZcdswpLkLooNHnODAN9r1Pxp3Y=
=sSlo
-----END PGP SIGNATURE-----

--dyfI+VVQf83HGbpw--