From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-vk1-f181.google.com (mail-vk1-f181.google.com [209.85.221.181]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 929BD7F for ; Thu, 5 Oct 2023 06:47:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bgdev-pl.20230601.gappssmtp.com header.i=@bgdev-pl.20230601.gappssmtp.com header.b="YrRVn0/X" Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=bgdev.pl Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=bgdev.pl Received: by mail-vk1-f181.google.com with SMTP id 71dfb90a1353d-499e2dd41ccso274584e0c.1 for ; Wed, 04 Oct 2023 23:47:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bgdev-pl.20230601.gappssmtp.com; s=20230601; t=1696488432; x=1697093232; darn=lists.linux.dev; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=0jW4B9vB7AjqRcLvugm3rYUGuuH5juE2C5rgpEuWVrM=; b=YrRVn0/XaNcjvGf+t3qN9Bg04GgkO7xg8dNeRzkDC5+n0NCqUytBgc5OWFyQPA1aRv 3x3dO2l9jKLpPAH1pt7zAFgQHoxzGv+nG91yq7vUabmfDOcKRegqsjpoTKrkwm5ZVxUQ wU1FdwPcIBOP89oklpaAE2x3T8LJcxCfmjLCz4EEPWmk5kRP3vuQbvEGMd8Gdfm5Q5WL rFVuFZsoWCncASkBcs8sI+nKUD0+sSUP3xuIvIEyl6hsppvlAvgCbTqsqsaNQsj6Xb3B +1FYdKNs5rprA1q92AD7klpERJXiXOe8PwUxzzCkM50F6mxKhlP5kFIHP7c4pE03AXFu XwTg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696488432; x=1697093232; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=0jW4B9vB7AjqRcLvugm3rYUGuuH5juE2C5rgpEuWVrM=; b=NXKTRqjmK08sEfobc9CvU8d9NRi4xNtw3YLQs61e+JigxBF4zxh7eUv3FAf1rPT+ja fOkpHf05TjNTc1pNKxBqWNAdTFGJbvzxpXMkxLnLZVS6AYhIX0C+fTlDoZXAU4nBPGNX gYgmdCalWoWRwKyhN7nci/OjGeLNzktua49gTisdzUq3xzS/rvSwQR7FeAr8EDg/6XWN pMsk45oM3Kf1eh1WVo7CtlE76QAqnbV1lm188Ikpol3UIk8O+ZSNyZlhaHq50sX69Fep WwijjTuACuZuEChIHdBjqArgxQ3Aq/OBLRUUAaoILk4fTqC+RaD7kSqw+78JW8Gy0ejq A6YQ== X-Gm-Message-State: AOJu0YxsJaojucYKeR8gWmocyYZjzaeu5Wf4Lt6Ll4Q0tiahouaE+U0c 2aKyEdBUPY5XLKqGRgQHpyKfORy/sWzbEI4lKIbiqA== X-Google-Smtp-Source: AGHT+IFNjDqGJnmk1DwdR80YBd34CbV4CUqePlsoUbIb8WmsHQmdMVqyYWfTXS6i9UHB3WXoJAeJK8L/e2xAWMF8p+M= X-Received: by 2002:a05:6122:221a:b0:49a:88a9:cac6 with SMTP id bb26-20020a056122221a00b0049a88a9cac6mr5164584vkb.11.1696488432143; Wed, 04 Oct 2023 23:47:12 -0700 (PDT) Precedence: bulk X-Mailing-List: timestamp@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20230911094443.14040-1-brgl@bgdev.pl> <7766de61-a046-3e17-1322-28bd7f1e61da@nvidia.com> <59ea74ba-b951-cf89-9d7f-bc7212ddb08a@nvidia.com> <7956eb81-014e-74cf-6630-b347a0b9d919@nvidia.com> In-Reply-To: <7956eb81-014e-74cf-6630-b347a0b9d919@nvidia.com> From: Bartosz Golaszewski Date: Thu, 5 Oct 2023 08:47:01 +0200 Message-ID: Subject: Re: [PATCH] hte: tegra194: improve the GPIO-related comment To: Dipen Patel Cc: Thierry Reding , Jonathan Hunter , timestamp@lists.linux.dev, linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org, Bartosz Golaszewski , Linus Walleij Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, Oct 4, 2023 at 10:24=E2=80=AFPM Dipen Patel wro= te: > > On 10/4/23 3:04 AM, Bartosz Golaszewski wrote: > > On Tue, Oct 3, 2023 at 6:42=E2=80=AFPM Dipen Patel = wrote: > >> > >> On 10/3/23 1:58 AM, Bartosz Golaszewski wrote: > >>> On Mon, Oct 2, 2023 at 6:27=E2=80=AFPM Dipen Patel wrote: > >>>> > >>>> On 10/2/23 1:33 AM, Bartosz Golaszewski wrote: > >>>>> On Fri, Sep 29, 2023 at 11:38=E2=80=AFPM Dipen Patel wrote: > >>>>>> > >>>>>> On 9/11/23 2:44 AM, Bartosz Golaszewski wrote: > >>>>>>> From: Bartosz Golaszewski > >>>>>>> > >>>>>>> Using any of the GPIO interfaces using the global numberspace is > >>>>>>> deprecated. Make it clear in the comment. > >>>>>>> > >>>>>>> Signed-off-by: Bartosz Golaszewski > >>>>>>> Acked-by: Linus Walleij > >>>>>>> --- > >>>>>>> This was part of a wider series but since this is independent, I'= m sending > >>>>>>> it separately. > >>>>>>> > >>>>>>> drivers/hte/hte-tegra194.c | 13 ++++++++----- > >>>>>>> 1 file changed, 8 insertions(+), 5 deletions(-) > >>>>>>> > >>>>>>> diff --git a/drivers/hte/hte-tegra194.c b/drivers/hte/hte-tegra19= 4.c > >>>>>>> index 6fe6897047ac..9fd3c00ff695 100644 > >>>>>>> --- a/drivers/hte/hte-tegra194.c > >>>>>>> +++ b/drivers/hte/hte-tegra194.c > >>>>>>> @@ -407,12 +407,15 @@ static int tegra_hte_line_xlate(struct hte_= chip *gc, > >>>>>>> return -EINVAL; > >>>>>>> > >>>>>>> /* > >>>>>>> + * GPIO consumers can access GPIOs in two ways: > >>>>>>> * > >>>>>>> - * There are two paths GPIO consumers can take as follows: > >>>>>>> - * 1) The consumer (gpiolib-cdev for example) which uses GP= IO global > >>>>>>> - * number which gets assigned run time. > >>>>>>> - * 2) The consumer passing GPIO from the DT which is assign= ed > >>>>>>> - * statically for example by using TEGRA194_AON_GPIO gpio D= T binding. > >>>>>>> + * 1) Using the global GPIO numberspace. > >>>>>>> + * > >>>>>>> + * This is the old, now DEPRECATED method and should not be= used in > >>>>>>> + * new code. TODO: Check if tegra is even concerned by this= . > >>>>>> This use case is to do namespace mapping from gpio subsystem to ht= e. Few doubts: > >>>>>> 1. What does deprecate mean here? Does gpio subsys not use global = space anymore? > >>>>> > >>>>> It does but we don't want to expose this to external users in any w= ay > >>>>> anymore (and haven't to for years). This is what deprecated means. > >>>>> Users should deal with opaque GPIO descriptors not global GPIO > >>>>> numberspace. > >>>>> > >>>>>> 2. If yes, what GPIO number is set when it comes from gpiolib-cdev= , as based on that I may have to > >>>>>> reflect in the mapping, tegra194_aon_gpio_map for example. > >>>>> > >>>>> Why DO you have to use a GPIO number though? If HTE needs just a > >>>>> number from some HTE numberspace (which in itself may be unnecessar= y) > >>>>> then why not just keep a local IDA for it? Do you have to know the > >>>>> GPIOs internal numbering scheme to make it work? > >>>> > >>> > >>> Dipen, > >>> > >>> Please set your mailer to wrap lines around at 80 characters as is > >>> customary on the mailing list. > >> > >> my email client misfired, will make sure. Thanks. > >>> > >>>> humm, overall, I just need to know which GPIO it is, for example, GP= IO controller X Port A GPIO number 3 > >>>> to do proper mapping. > >>>> Continuing from above example, the hte driver gets: > >>>> - GPIO Controller X from DT node > >>>> - the rest details in current code gets it from [1] and [2] > >>>> > >>>> If there is alternate method exists, I would like to explore. I thin= k IDA will not help in this case as ID assigned > >>>> does not hold meaning in this context. > >>>> > >>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.g= it/tree/drivers/gpio/gpiolib-cdev.c?h=3Dv6.6-rc3#n760 > >>> > >>> Here: any reason why we have to translate the desc to the global GPIO > >>> numberspace? Can we just pass the descriptor pointer directly to the > >>> HTE subsystem? > >> Sure, if from GPIO descriptor with combination of any helper APIs from > >> the GPIO subsystem can help identify the GPIO pin, we can eliminate th= e need > >> to pass global number (I assume gpio desc > >> can be only accessed/manipulated using GPIO subsystem APIs) > >> > >>> > >>>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.g= it/tree/drivers/hte/hte-tegra194.c?h=3Dv6.6-rc3#n421 > >>> > >>> I still don't understand why you need to know the GPIO base? I'm not > >>> quite sure what the role of line_id is in this driver. Is it only to > >>> index the array? > >>> > >>> Please bear with me, I don't know this subsystem very well. > >> > >> Sure, no problem. Let me see if I am able to elaborate: > >> > >> 1. Map arrays' indexes are GPIO offsets so to avoid having > >> the extra field for the GPIO numbers. > >> > >> 2. The HTE driver needs to know exact GPIO to enable corresponding bit= s > >> in its registers. For example, hte register bit 3 would correspond to > >> GPIO 6 of GPIO controller X. If gpio descriptor is passed here, I thin= k > >> I would need to do some conversions to identify the GPIO to enable > >> corresponding register bits. In the current scheme of things, > >> I though it was easier to identify passing the output of the desc_to_g= pio* API. > >> > >> 3. Since GPIO global space is runtime, need base to calculate the offs= et > >> where offset does not change. So in the above example, gpio cdev would= pass > >> 306 and HTE does simple conversion from the base, ie. 306 - 300 =3D 6. > >> Now 6 will serve as pin number as map array index to find the register= . > >> > >> 4. Overall, I rely on base + global number to do namespace conversion > >> between gpio and hte subsys as far as gpio-cdev use case is concerned. > >> > > > > Ok, so you *don't* need the global numbers, just controller-relative > > Thats correct.... > > > offsets. This makes sense. This ties nicely into my plan for removing > > all accesses to gpio_chip except for GPIO providers. > > > > Looking at the tegra dts I'm surprised that the GPIO controllers that > > use the HTE don't take the hte_lic or hte_aon phandles as arguments. > > How exactly do they resolve the HTE device to use for timestamping? > gpiochip added few callbacks during hte subsystem patch series to manipul= ate > gpio line that needs hte. So it does not need any handle as of now for th= e > tegra series of gpio controllers at least. > > > > > In any case, I think this commit is still correct. > Agreed. > > > > Bart > > > >>> > >>> Bart > >>> > >>>> > >>>>> > >>>>> Bart > >>>>> > >>>>>>> + * > >>>>>>> + * 2) Using GPIO descriptors that can be assigned to consum= er devices > >>>>>>> + * using device-tree, ACPI or lookup tables. > >>>>>>> * > >>>>>>> * The code below addresses both the consumer use cases and= maps into > >>>>>>> * HTE/GTE namespace. > >>>>>> > >>>> > >> > Reviewed-by: Dipen Patel I think you can just take it through your tree, GPIO does not depend on this change in any way. Bart