[v3,2/2] gpio: Get rid of gpio_hog_probe_all()

From: Marek Vasut <marex@denx.de>

The gpio_hog_probe_all() functionality can be perfectly well replaced by
DM_FLAG_PROBE_AFTER_BIND DM flag, which would trigger .probe() callback
of each GPIO hog driver instance after .bind() and thus configure the
hogged GPIO accordingly.

Signed-off-by: Marek Vasut <marex@denx.de>
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
Reviewed-by: Samuel Holland <samuel@sholland.org>
---

v3:
 - removed DM_FLAG_PRE_RELOC flag since it is now handled by patch 1/2
 in this series, in the DM core,

v2:
 - added missing DM_FLAG_PRE_RELOC flag on the gpio-hog device,
 - added comments for flags setting,
 - tested on a PX30 ringneck where the gpio-hog is necessary in SPL to
 be able to load U-Boot proper from eMMC when booting SPL from SD card,

 common/board_r.c           |  3 ---
 common/spl/spl.c           |  3 ---
 doc/README.gpio            |  6 ++----
 drivers/gpio/gpio-uclass.c | 31 ++++++++-----------------------
 include/asm-generic/gpio.h |  8 --------
 5 files changed, 10 insertions(+), 41 deletions(-)

Hi Tom 

This patch is landing in the mailing list since a while. 
Do you expect to merge it in master or in next branch soon ?
This patch will be useful for STM32MP SoC in order to clean according machine code.

Thanks
Patrice


On 9/22/22 17:53, Quentin Schulz wrote:
> From: Marek Vasut <marex@denx.de>
> 
> The gpio_hog_probe_all() functionality can be perfectly well replaced by
> DM_FLAG_PROBE_AFTER_BIND DM flag, which would trigger .probe() callback
> of each GPIO hog driver instance after .bind() and thus configure the
> hogged GPIO accordingly.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> Reviewed-by: Samuel Holland <samuel@sholland.org>
> ---
> 
> v3:
>  - removed DM_FLAG_PRE_RELOC flag since it is now handled by patch 1/2
>  in this series, in the DM core,
> 
> v2:
>  - added missing DM_FLAG_PRE_RELOC flag on the gpio-hog device,
>  - added comments for flags setting,
>  - tested on a PX30 ringneck where the gpio-hog is necessary in SPL to
>  be able to load U-Boot proper from eMMC when booting SPL from SD card,
> 
>  common/board_r.c           |  3 ---
>  common/spl/spl.c           |  3 ---
>  doc/README.gpio            |  6 ++----
>  drivers/gpio/gpio-uclass.c | 31 ++++++++-----------------------
>  include/asm-generic/gpio.h |  8 --------
>  5 files changed, 10 insertions(+), 41 deletions(-)
> 
> diff --git a/common/board_r.c b/common/board_r.c
> index 56eb60fa27..c556aa5a07 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -750,9 +750,6 @@ static init_fnc_t init_sequence_r[] = {
>  	initr_status_led,
>  #endif
>  	/* PPC has a udelay(20) here dating from 2002. Why? */
> -#if defined(CONFIG_GPIO_HOG)
> -	gpio_hog_probe_all,
> -#endif
>  #ifdef CONFIG_BOARD_LATE_INIT
>  	board_late_init,
>  #endif
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index 29e0898f03..683e0dfc52 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -770,9 +770,6 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>  		}
>  	}
>  
> -	if (CONFIG_IS_ENABLED(GPIO_HOG))
> -		gpio_hog_probe_all();
> -
>  #if CONFIG_IS_ENABLED(BOARD_INIT)
>  	spl_board_init();
>  #endif
> diff --git a/doc/README.gpio b/doc/README.gpio
> index 548ff37b8c..d253f654fa 100644
> --- a/doc/README.gpio
> +++ b/doc/README.gpio
> @@ -2,10 +2,8 @@
>  GPIO hog (CONFIG_GPIO_HOG)
>  --------
>  
> -All the GPIO hog are initialized in gpio_hog_probe_all() function called in
> -board_r.c just before board_late_init() but you can also acces directly to
> -the gpio with gpio_hog_lookup_name().
> -
> +All the GPIO hog are initialized using DM_FLAG_PROBE_AFTER_BIND DM flag
> +after bind().
>  
>  Example, for the device tree:
>  
> diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
> index 0ed32b7217..b08e482ab3 100644
> --- a/drivers/gpio/gpio-uclass.c
> +++ b/drivers/gpio/gpio-uclass.c
> @@ -315,34 +315,11 @@ static int gpio_hog_probe(struct udevice *dev)
>  	return 0;
>  }
>  
> -int gpio_hog_probe_all(void)
> -{
> -	struct udevice *dev;
> -	int ret;
> -	int retval = 0;
> -
> -	for (uclass_first_device(UCLASS_NOP, &dev);
> -	     dev;
> -	     uclass_find_next_device(&dev)) {
> -		if (dev->driver == DM_DRIVER_GET(gpio_hog)) {
> -			ret = device_probe(dev);
> -			if (ret) {
> -				printf("Failed to probe device %s err: %d\n",
> -				       dev->name, ret);
> -				retval = ret;
> -			}
> -		}
> -	}
> -
> -	return retval;
> -}
> -
>  int gpio_hog_lookup_name(const char *name, struct gpio_desc **desc)
>  {
>  	struct udevice *dev;
>  
>  	*desc = NULL;
> -	gpio_hog_probe_all();
>  	if (!uclass_get_device_by_name(UCLASS_NOP, name, &dev)) {
>  		struct gpio_hog_priv *priv = dev_get_priv(dev);
>  
> @@ -1503,9 +1480,17 @@ static int gpio_post_bind(struct udevice *dev)
>  								 &child);
>  				if (ret)
>  					return ret;
> +
> +				/*
> +				 * Make sure gpio-hogs are probed after bind
> +				 * since hogs can be essential to the hardware
> +				 * system.
> +				 */
> +				dev_or_flags(child, DM_FLAG_PROBE_AFTER_BIND);
>  			}
>  		}
>  	}
> +
>  	return 0;
>  }
>  
> diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> index 81f63f06f1..e56d3777ae 100644
> --- a/include/asm-generic/gpio.h
> +++ b/include/asm-generic/gpio.h
> @@ -460,14 +460,6 @@ int dm_gpio_lookup_name(const char *name, struct gpio_desc *desc);
>   */
>  int gpio_hog_lookup_name(const char *name, struct gpio_desc **desc);
>  
> -/**
> - * gpio_hog_probe_all() - probe all gpio devices with
> - * gpio-hog subnodes.
> - *
> - * @return:	Returns return value from device_probe()
> - */
> -int gpio_hog_probe_all(void);
> -
>  /**
>   * gpio_lookup_name - Look up a GPIO name and return its details
>   *

Hi Patrice,

On 1/3/23 14:35, Patrice CHOTARD wrote:
> 
> Hi Tom
> 
> This patch is landing in the mailing list since a while.
> Do you expect to merge it in master or in next branch soon ?
> This patch will be useful for STM32MP SoC in order to clean according machine code.
> 

The main issue here is that Simon would like a test case for the first 
patch in the series and I wasn't able to add tests to the sandbox to 
guarantee there's no regression in the future, hence why it was kinda 
parked on the ML without much work on it.

 From my U-Boot IRC archive:
2022-09-26 15:22:58     ^[[94m^[[0m^[[1m^[[38;5;7m^[[49mqschulz 
^[[0msjg1: it seems I'm unable to write a unit test for this dm patch
2022-09-26 15:24:33     ^[[94m^[[0m^[[1m^[[38;5;7m^[[49mqschulz 
^[[0msjg1: https://paste.ack.tf/dc8598 is what I came up with, but 
impossible to actually run this test
2022-09-26 15:25:23     ^[[94m^[[0m^[[1m^[[38;5;7m^[[49mqschulz ^[[0mway 
too many changes to be able to even compile this with sandbox_spl_defconfig
2022-09-26 15:25:47     ^[[94m^[[0m^[[1m^[[38;5;7m^[[49mqschulz ^[[0mand 
with sandbox_defconfig, ./u-boot -T -c "ut dm fdt" does not run it either
2022-09-26 15:27:18     ^[[94m^[[0m^[[1m^[[38;5;7m^[[49mqschulz ^[[0mso 
i'm not against some guidance on how to write a unit test for that patch
2022-09-26 15:28:52     ^[[94m^[[0m^[[1m^[[38;5;7m^[[49mqschulz 
^[[0mwhat we want to test is that in pre-reloc, a driver with 
DM_FLAG_PROBE_AFTER_BIND flag but not DM_FLAG_PRE_RELOC gets probed if 
it is bound to a device tree node where u-boot,dm-pre-reloc is set

Cheers,
Quentin

On 1/3/23 14:50, Quentin Schulz wrote:
> Hi Patrice,
> 
> On 1/3/23 14:35, Patrice CHOTARD wrote:
>>
>> Hi Tom
>>
>> This patch is landing in the mailing list since a while.
>> Do you expect to merge it in master or in next branch soon ?
>> This patch will be useful for STM32MP SoC in order to clean according 
>> machine code.
>>
> 
> The main issue here is that Simon would like a test case for the first 
> patch in the series and I wasn't able to add tests to the sandbox to 
> guarantee there's no regression in the future, hence why it was kinda 
> parked on the ML without much work on it.

Hmmm, seems there is little input on how to write the test or even what 
to test, so maybe this should be just merged, esp. if it is useful.

On Tue, Jan 03, 2023 at 02:35:00PM +0100, Patrice CHOTARD wrote:

> 
> Hi Tom 
> 
> This patch is landing in the mailing list since a while. 
> Do you expect to merge it in master or in next branch soon ?
> This patch will be useful for STM32MP SoC in order to clean according machine code.

Thanks for following up.  In addition to what Quentin said, there were a
few related solutions to a problem and it wasn't quite clear to me if
this was also still needed.  I'll keep an eye out for v4.

On Tue, Jan 03, 2023 at 04:28:13PM +0100, Marek Vasut wrote:
> On 1/3/23 14:50, Quentin Schulz wrote:
> > Hi Patrice,
> > 
> > On 1/3/23 14:35, Patrice CHOTARD wrote:
> > > 
> > > Hi Tom
> > > 
> > > This patch is landing in the mailing list since a while.
> > > Do you expect to merge it in master or in next branch soon ?
> > > This patch will be useful for STM32MP SoC in order to clean
> > > according machine code.
> > > 
> > 
> > The main issue here is that Simon would like a test case for the first
> > patch in the series and I wasn't able to add tests to the sandbox to
> > guarantee there's no regression in the future, hence why it was kinda
> > parked on the ML without much work on it.
> 
> Hmmm, seems there is little input on how to write the test or even what to
> test, so maybe this should be just merged, esp. if it is useful.

What's missing from
https://u-boot.readthedocs.io/en/latest/develop/tests_writing.html for
this case?

On Thu, Sep 22, 2022 at 05:53:26PM +0200, Quentin Schulz wrote:

> From: Marek Vasut <marex@denx.de>
> 
> The gpio_hog_probe_all() functionality can be perfectly well replaced by
> DM_FLAG_PROBE_AFTER_BIND DM flag, which would trigger .probe() callback
> of each GPIO hog driver instance after .bind() and thus configure the
> hogged GPIO accordingly.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> Reviewed-by: Samuel Holland <samuel@sholland.org>

With the expectation that a test will be written later, applied to
u-boot/master, thanks!