[PATCH] gpio: Get rid of gpio_hog_probe_all()

[Quentin Schulz]

Hi all,

Just so there's a written summary on the ML rather than only on IRC.

On 9/20/22 11:53, Marek Vasut wrote:
> On 9/20/22 11:00, Quentin Schulz wrote:
>> Hi Marek,
> 
> Hi,
> 
>> On 9/19/22 21:45, Marek Vasut wrote:
>>> 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 at denx.de>
>>
>> This patch breaks the U-Boot proper loading fallback mechanism on Puma 
>> RK3399.
>>
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_u-2Dboot_20220915091432.789294-2D1-2Dfoss-2Buboot-400leil.net_&d=DwICaQ&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=aMC9n5Cgsl79NAwWBVdx4n9MNHT7Jcm1g4sKww821Kd8JuvW1wjvVHLLPU2XsXVe&s=YbHd2jLJ2foRj_fTpj_PNmI5mXcUi5wGpzC3oL_rcVs&e=  is the base I used, on top of commit d6a03711fd with your patch applied on top.
>>
>> I need the GPIO hogs to be probed before the SPL starts looking for 
>> storage media for U-Boot proper because the GPIO hog is necessary on 
>> the HW level for the eMMC and SPI-NOR flash to be usable.
> 
> That _should_ still work.
> 
> Look at common/spl/spl.c board_init_r() calls spl_init() calls 
> spl_common_init() -> dm_init_and_scan() . That should bind and probe the 
> GPIO hogs for you . The old gpio_hog_probe_all() happened AFTER all this.
> 
> Can you use e.g. dm_dump_all() call in or around spl_board_init() to 
> check whether the hogs got probed in SPL ?
> 

After some debugging with Marek, we found out that DM_FLAG_PRE_RELOC 
flag is not set on the gpio hogs and thus, dm_probe_devices function in 
drivers/core/root.c will not call device_probe() function.

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index 32df0448a7..37a8706746 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -1481,7 +1481,7 @@ static int gpio_post_bind(struct udevice *dev)
                                 if (ret)
                                         return ret;

-                               dev_or_flags(child, 
DM_FLAG_PROBE_AFTER_BIND);
+                               dev_or_flags(child, 
DM_FLAG_PROBE_AFTER_BIND | DM_FLAG_PRE_RELOC);
                         }
                 }
         }

made it work but it is not good enough (the flag needs to be added only 
if the DT property is there for example).

One way could be to add this flag in a post_bind callback of the 
gpio-hog driver.

But I believe a better way would have the DM code actually handle this 
on its own instead of relying on subsystems/drivers to do it.

Something like:

diff --git a/drivers/core/device.c b/drivers/core/device.c
index d9ce546c0c..103ec47b88 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -174,6 +174,10 @@ static int device_bind_common(struct udevice 
*parent, const struct driver *drv,
  		if (ret)
  			goto fail_child_post_bind;
  	}
+
+	if (ofnode_pre_reloc(node))
+		dev_or_flags(dev, DM_FLAG_PRE_RELOC);
+
  	if (uc->uc_drv->post_bind) {
  		ret = uc->uc_drv->post_bind(dev);
  		if (ret)

but with the knowledge of pre_reloc_only flag I guess? I have no 
experience in the DM code so maybe someone will have a better idea/quick 
implementation, so that this patch can actually be merged without 
breaking stuff :)

>> Basically, we have a switch on the board disabling eMMC and SPI-NOR so 
>> that we can boot from SD card, but we want to be able to load U-Boot 
>> proper from eMMC/SPI-NOR. The GPIO hog overrides this HW switch. Use 
>> case is to recover from a corrupted SPL without touching U-Boot proper.
> 
> I don't quite get this -- isn't board_boot_order() or spl_boot_device() 
> used for the purpose of selecting boot media , including fallback ?
> 
> [...]

In short, we have a switch on the devkit which allows to cut the power 
and clock lines to the eMMC and put the SPI-NOR flash in HOLD mode 
(reset basically).

We then have a GPIO we can control with SW that allows to electrically 
override this switch on the devkit so that at runtime we can enable eMMC 
and SPI-NOR flash.

The boot selection is handled by U-Boot (Rockchip mach directory deals 
with spl_boot_order), but the devices aren't physically usable until 
this GPIO is used to electrically override the switch on the devkit.

The use case being: I manually (and electrically) disable eMMC and SPI 
flash with the switch to boot from SD Card, then if U-Boot proper on the 
SD card is corrupted or not present, fallback to loading it from eMMC or 
SPI flash.

Cheers,
Quentin