All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] arm: gpio: Add set_flags callback to the Tegra gpio driver
@ 2025-09-22 14:16 Lukasz Majewski
  2025-09-26 11:59 ` Svyatoslav Ryhel
  0 siblings, 1 reply; 8+ messages in thread
From: Lukasz Majewski @ 2025-09-22 14:16 UTC (permalink / raw)
  To: Thierry Reding, Svyatoslav Ryhel
  Cc: Tom Rini, u-boot, Simon Glass, Lukasz Majewski

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(-)

diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c
index 3d1e18854f2..85f2d28ee73 100644
--- a/drivers/gpio/tegra_gpio.c
+++ b/drivers/gpio/tegra_gpio.c
@@ -182,21 +182,6 @@ static int tegra_gpio_get_value(struct udevice *dev, unsigned offset)
 	return (val >> GPIO_BIT(gpio)) & 1;
 }
 
-/* write GPIO OUT value to pin 'gpio' */
-static int tegra_gpio_set_value(struct udevice *dev, unsigned offset, int value)
-{
-	struct tegra_port_info *state = dev_get_priv(dev);
-	int gpio = state->base_gpio + offset;
-
-	debug("gpio_set_value: pin = %d (port %d:bit %d), value = %d\n",
-	      gpio, GPIO_FULLPORT(gpio), GPIO_BIT(gpio), value);
-
-	/* Configure GPIO output value. */
-	set_level(gpio, value);
-
-	return 0;
-}
-
 void gpio_config_table(const struct tegra_gpio_config *config, int len)
 {
 	int i;
@@ -258,14 +243,36 @@ static int tegra_gpio_rfree(struct udevice *dev, unsigned int offset)
 	return 0;
 }
 
+static int tegra_gpio_set_flags(struct udevice *dev, unsigned int offset,
+				ulong flags)
+{
+	struct tegra_port_info *state = dev_get_priv(dev);
+	int gpio = state->base_gpio + offset;
+
+	debug("gpio_set_flags: pin = %d (port %d:bit %d), flag = 0x%lx\n",
+	      gpio, GPIO_FULLPORT(gpio), GPIO_BIT(gpio), flags);
+
+	if (flags & GPIOD_IS_AF) {
+		set_config(gpio, CFG_SFIO);
+	} else if (flags & GPIOD_IS_IN) {
+		return tegra_gpio_direction_input(dev, offset);
+	} else if (flags & GPIOD_IS_OUT) {
+		bool value = flags & GPIOD_IS_OUT_ACTIVE;
+
+		return tegra_gpio_direction_output(dev, offset, value);
+	}
+
+	return 0;
+}
+
 static const struct dm_gpio_ops gpio_tegra_ops = {
 	.direction_input	= tegra_gpio_direction_input,
 	.direction_output	= tegra_gpio_direction_output,
 	.get_value		= tegra_gpio_get_value,
-	.set_value		= tegra_gpio_set_value,
 	.get_function		= tegra_gpio_get_function,
 	.xlate			= tegra_gpio_xlate,
 	.rfree			= tegra_gpio_rfree,
+	.set_flags		= tegra_gpio_set_flags,
 };
 
 /*
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] arm: gpio: Add set_flags callback to the Tegra gpio driver
  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
  0 siblings, 1 reply; 8+ messages in thread
From: Svyatoslav Ryhel @ 2025-09-26 11:59 UTC (permalink / raw)
  To: Lukasz Majewski; +Cc: Thierry Reding, Tom Rini, u-boot, Simon Glass

пн, 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 this and I will return
to it but that may not be soon unfortunately. 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.

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] arm: gpio: Add set_flags callback to the Tegra gpio driver
  2025-09-26 11:59 ` Svyatoslav Ryhel
@ 2025-09-29  7:23   ` Łukasz Majewski
  2025-09-29  7:28     ` Svyatoslav Ryhel
  0 siblings, 1 reply; 8+ messages in thread
From: Łukasz Majewski @ 2025-09-29  7:23 UTC (permalink / raw)
  To: Svyatoslav Ryhel; +Cc: Thierry Reding, Tom Rini, u-boot, Simon Glass

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?

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

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

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,

Lukasz Majewski

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] arm: gpio: Add set_flags callback to the Tegra gpio driver
  2025-09-29  7:23   ` Łukasz Majewski
@ 2025-09-29  7:28     ` Svyatoslav Ryhel
  2025-09-29 10:22       ` Łukasz Majewski
  0 siblings, 1 reply; 8+ messages in thread
From: Svyatoslav Ryhel @ 2025-09-29  7:28 UTC (permalink / raw)
  To: Łukasz Majewski; +Cc: Thierry Reding, Tom Rini, u-boot, Simon Glass

пн, 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

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

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

> 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,
>
> Lukasz Majewski
>
> --
> Nabla Software Engineering GmbH
> HRB 40522 Augsburg
> Phone: +49 821 45592596
> E-Mail: office@nabladev.com
> Geschftsfhrer : Stefano Babic

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] arm: gpio: Add set_flags callback to the Tegra gpio driver
  2025-09-29  7:28     ` Svyatoslav Ryhel
@ 2025-09-29 10:22       ` Łukasz Majewski
  2025-10-03 17:51         ` Svyatoslav Ryhel
  0 siblings, 1 reply; 8+ messages in thread
From: Łukasz Majewski @ 2025-09-29 10:22 UTC (permalink / raw)
  To: Svyatoslav Ryhel; +Cc: Thierry Reding, Tom Rini, u-boot, Simon Glass

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,
> >
> > Lukasz Majewski
> >
> > --
> > Nabla Software Engineering GmbH
> > HRB 40522 Augsburg
> > Phone: +49 821 45592596
> > E-Mail: office@nabladev.com
> > Geschftsfhrer : Stefano Babic  



-- 
Best regards,

Lukasz Majewski

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] arm: gpio: Add set_flags callback to the Tegra gpio driver
  2025-09-29 10:22       ` Łukasz Majewski
@ 2025-10-03 17:51         ` Svyatoslav Ryhel
  2025-10-06  7:15           ` Łukasz Majewski
  0 siblings, 1 reply; 8+ messages in thread
From: Svyatoslav Ryhel @ 2025-10-03 17:51 UTC (permalink / raw)
  To: Łukasz Majewski; +Cc: Thierry Reding, Tom Rini, u-boot, Simon Glass

пн, 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).

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] arm: gpio: Add set_flags callback to the Tegra gpio driver
  2025-10-03 17:51         ` Svyatoslav Ryhel
@ 2025-10-06  7:15           ` Łukasz Majewski
  2025-10-06  7:18             ` Svyatoslav Ryhel
  0 siblings, 1 reply; 8+ messages in thread
From: Łukasz Majewski @ 2025-10-06  7:15 UTC (permalink / raw)
  To: Svyatoslav Ryhel; +Cc: Thierry Reding, Tom Rini, u-boot, Simon Glass

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] arm: gpio: Add set_flags callback to the Tegra gpio driver
  2025-10-06  7:15           ` Łukasz Majewski
@ 2025-10-06  7:18             ` Svyatoslav Ryhel
  0 siblings, 0 replies; 8+ messages in thread
From: Svyatoslav Ryhel @ 2025-10-06  7:18 UTC (permalink / raw)
  To: Łukasz Majewski; +Cc: Thierry Reding, Tom Rini, u-boot, Simon Glass

пн, 6 жовт. 2025 р. о 10:15 Łukasz Majewski <lukma@nabladev.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>
>

Reviewed-by: Svyatoslav Ryhel <clamor95@gmail.com>
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>

Applied to u-boot-tegra/staging, thank you!

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-10-06  7:18 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2025-10-06  7:18             ` Svyatoslav Ryhel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.