Re: [PATCH] hte: tegra194: improve the GPIO-related comment

[Bartosz Golaszewski]

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.

> 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?

> [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.

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.
> >>
>