On Wed, Oct 4, 2023 at 10:24 PM Dipen Patel <dipenp@xxxxxxxxxx> wrote: > > On 10/4/23 3:04 AM, Bartosz Golaszewski wrote: > > On Tue, Oct 3, 2023 at 6:42 PM Dipen Patel <dipenp@xxxxxxxxxx> wrote: > >> > >> On 10/3/23 1:58 AM, Bartosz Golaszewski wrote: > >>> On Mon, Oct 2, 2023 at 6:27 PM Dipen Patel <dipenp@xxxxxxxxxx> wrote: > >>>> > >>>> On 10/2/23 1:33 AM, Bartosz Golaszewski wrote: > >>>>> On Fri, Sep 29, 2023 at 11:38 PM Dipen Patel <dipenp@xxxxxxxxxx> wrote: > >>>>>> > >>>>>> On 9/11/23 2:44 AM, Bartosz Golaszewski wrote: > >>>>>>> From: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx> > >>>>>>> > >>>>>>> Using any of the GPIO interfaces using the global numberspace is > >>>>>>> deprecated. Make it clear in the comment. > >>>>>>> > >>>>>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx> > >>>>>>> Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx> > >>>>>>> --- > >>>>>>> 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-tegra194.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 GPIO global > >>>>>>> - * number which gets assigned run time. > >>>>>>> - * 2) The consumer passing GPIO from the DT which is assigned > >>>>>>> - * statically for example by using TEGRA194_AON_GPIO gpio DT 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 hte. 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 way > >>>>> 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 unnecessary) > >>>>> 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, GPIO 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 think 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.git/tree/drivers/gpio/gpiolib-cdev.c?h=v6.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 the 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.git/tree/drivers/hte/hte-tegra194.c?h=v6.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 bits > >> 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 think > >> 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_gpio* API. > >> > >> 3. Since GPIO global space is runtime, need base to calculate the offset > >> 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 = 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 manipulate > gpio line that needs hte. So it does not need any handle as of now for the > 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 consumer 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 <dipenp@xxxxxxxxxx> I think you can just take it through your tree, GPIO does not depend on this change in any way. Bart
