U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Łukasz Majewski" <lukma@nabladev.com>
To: Svyatoslav Ryhel <clamor95@gmail.com>
Cc: Thierry Reding <treding@nvidia.com>,
	Tom Rini <trini@konsulko.com>,
	u-boot@lists.denx.de, Simon Glass <sjg@chromium.org>
Subject: Re: [PATCH v2] arm: gpio: Add set_flags callback to the Tegra gpio driver
Date: Mon, 6 Oct 2025 09:15:25 +0200	[thread overview]
Message-ID: <20251006091525.04cecc64@wsk> (raw)
In-Reply-To: <CAPVz0n0wU8e_PBQ7V9_6pQam20WKv9PYhFfr8F+nXb-mSKRztQ@mail.gmail.com>

Hi Svyatoslav,

> пн, 29 вер. 2025 р. о 13:22 Łukasz Majewski <lukma@nabladev.com> пише:
> >
> > Hi Svyatoslav,
> >  
> > > пн, 29 вер. 2025 р. о 10:23 Łukasz Majewski <lukma@nabladev.com>
> > > пише:  
> > > >
> > > > Hi Svyatoslav,
> > > >  
> > > > > пн, 22 вер. 2025 р. о 17:16 Lukasz Majewski
> > > > > <lukma@nabladev.com> пише:  
> > > > > >
> > > > > > This patch adds support for the .set_flags callback.
> > > > > > For now following flags are supported:
> > > > > > - GPIOD_IS_AF (i.e. "alternate function").
> > > > > > - GPIOD_IS_IN
> > > > > > - GPIOD_IS_OUT
> > > > > >
> > > > > > Currently, the .set_flags in gpio-uclass.c (function
> > > > > > dm_gpio_set_value()) is used before .set_value callback, so
> > > > > > functionally replaces it. As a result the corresponding
> > > > > > tegra_gpio_set_value() can be removed.
> > > > > >
> > > > > > Signed-off-by: Lukasz Majewski <lukma@nabladev.com>
> > > > > > ---
> > > > > >
> > > > > > Changes for v2:
> > > > > > - Fix the format specifier for flags in debug() function
> > > > > > - Update commit message
> > > > > > - Remove tegra_gpio_set_value() method (as it is
> > > > > > functionally replaced by set_value()
> > > > > > - Prevent from returning errors when flags = 0 (problem with
> > > > > > e.g. I2C GPIO support)
> > > > > > ---
> > > > > >  drivers/gpio/tegra_gpio.c | 39
> > > > > > +++++++++++++++++++++++---------------- 1 file changed, 23
> > > > > > insertions(+), 16 deletions(-)  
> > > > >
> > > > > Lukasz, thank you for this change I would really like to see
> > > > > it in mainline. ATM I have no capabilities to debug  
> > > >
> > > > You mean test if it works?
> > > >  
> > > > > this and I will return
> > > > > to it but that may not be soon unfortunately.  
> > > >
> > > > This is a bit problematic for me, as:
> > > >
> > > > 1. Some future work depends on it (more details below).
> > > > 2. The "set_flags()" callback seems like being now the one to be
> > > > advised to being implemented and used.
> > > >
> > > >
> > > >
> > > > Can you predict when you will be able to come back to this
> > > > task? Is this the matter of weeks or months?
> > > >  
> > >
> > > Weeks maybe, I will try to look into this when I have some spare
> > > time 
> >
> > Ok.
> >  
> > > > > In case you really need
> > > > > switch GPIO back to SFIO and Linux cannot handle this, you an
> > > > > use dm_gpio_free to release gpios in board_preboot_os in the
> > > > > board as a temporary measure.  
> > > >
> > > > Yes, this is a temporary solution - i.e. dm_gpio_free() is
> > > > supposed to "free" the pin. IMHO, the set_flags() shall be
> > > > used, as I in fact do want to set the CFG_SFIO flag, not
> > > > release the gpio. 
> > >
> > > dm_gpio_free in tegra case calls CFG_SFIO  
> >
> > Yes, but as you said - this is not the "proper" solution.
> >  
> > >  
> > > > >
> > > > > Overall issue you are describing is not u-boot's it is
> > > > > kernels, kernel must reconfigure gpios for proper work
> > > > > regardless of their previous state. If it is not the case,
> > > > > then kernel device configuration is incomplete or wrong.  
> > > >
> > > > This is how the pinmux in Linux for Tegra is written. You can
> > > > setup the PAD parameters, but you cannot set the pin's function.
> > > >  
> > >
> > > WDYM? Tegra30+ has per-pin configuration, this includes FUNCTION,
> > > direction, tristate and pull for all pins  
> >
> > With DTS pinmux description, you cannot change the PAD function
> > (special vs gpio) in Linux. It is assumed that bootloader would set
> > the function (which, yes, is IMHO a bug).
> >  
> > >  
> > > > In other words - you have to do it in bootloader (u-boot in this
> > > > case).
> > > >
> > > > In my case - customer uses the pin in u-boot as GPIO to check
> > > > carrier board version, then the same pin in Linux is going to be
> > > > used as special function one.
> > > >
> > > > Unfortunately, above "reconfiguration" cannot be done in Linux.
> > > >
> > > > As other "rework" of gpio code depends on it (use dm) - I would
> > > > like to postpone my work until this functionality is available
> > > > in mainline (and hence avoid "temporal solutions").
> > > >
> > > > --
> > > > Best regards,  
> 
> Try this
> https://source.denx.de/u-boot/custodians/u-boot-tegra/-/commit/d95d942b6e796429342845d153d95330f5257cdd
> I have combined your v1 and v2 and added a few tweaks. It works on
> star and olympus (tegra20), p895 and grouper (tegra30), tegratab and
> tf701t (tegra114) and mocha (tegra124).

I've tested it on Colibri T30. No issues observed.

Thanks for your tweaks.

Please add mine:
Tested-by: Łukasz Majewski <lukma@nabladev.com>


-- 
Best regards,

Lukasz Majewski

--
Nabla Software Engineering GmbH
HRB 40522 Augsburg
Phone: +49 821 45592596
E-Mail: office@nabladev.com
Geschftsfhrer : Stefano Babic

  reply	other threads:[~2025-10-06  7:15 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-22 14:16 [PATCH v2] arm: gpio: Add set_flags callback to the Tegra gpio driver Lukasz Majewski
2025-09-26 11:59 ` Svyatoslav Ryhel
2025-09-29  7:23   ` Łukasz Majewski
2025-09-29  7:28     ` Svyatoslav Ryhel
2025-09-29 10:22       ` Łukasz Majewski
2025-10-03 17:51         ` Svyatoslav Ryhel
2025-10-06  7:15           ` Łukasz Majewski [this message]
2025-10-06  7:18             ` Svyatoslav Ryhel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251006091525.04cecc64@wsk \
    --to=lukma@nabladev.com \
    --cc=clamor95@gmail.com \
    --cc=sjg@chromium.org \
    --cc=treding@nvidia.com \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox