[PATCH 1/3] dm: core: Add DM_FLAG_PROBE_AFTER_BIND flag

[Marek Vasut]

On 4/23/22 02:16, Sean Anderson wrote:
> On 4/22/22 7:07 PM, Marek Vasut wrote:
>> On 4/23/22 00:45, Sean Anderson wrote:
>>> On 4/22/22 9:52 AM, Marek Vasut wrote:
>>>> On 4/22/22 15:49, Sean Anderson wrote:
>>>>> On 4/22/22 9:15 AM, Marek Vasut wrote:
>>>>>> Introduce DM_FLAG_PROBE_AFTER_BIND flag, which can be set by 
>>>>>> driver or
>>>>>> uclass in .bind(), to indicate such driver instance should be 
>>>>>> probe()d
>>>>>> once binding of all devices is complete.
>>>>>>
>>>>>> This is useful in case the driver determines that hardware 
>>>>>> initialization
>>>>>> is mandatory on boot, and such initialization happens only in 
>>>>>> probe().
>>>>>> This also solves the inability to call device_probe() from .bind().
>>>>>
>>>>> So why not add an init_leds function to init_sequence_r?
>>>>
>>>> Because this flag is generic solution and not hack specific to LEDs.
>>>
>>> Isn't init_sequence_r the generic solution?
>>
>> No, init_sequence_r is just a list of functions that gets called early 
>> on during boot to bring U-Boot up. One of the functions is initr_dm(), 
>> DM init, which brings up the driver model. The init_sequence_r has 
>> nothing to do with driver binding or probing though, that's the DM job.
>>
>>> Every device gets probed that way (or directly as a result of a 
>>> command).
>>
>> No device probe gets triggered via init_sequence_r.
>> There are many ad-hoc device_probe() calls in the various 
>> init_sequence_r function implementation,
> 
> Every device in U-Boot is probed in one of three ways
> 
> - As a dependency of another device (clocks, pinmuxes, gpios, etc.)
> - Directly on the command line (most boot devices)
> - From an init_sequence (timers, watchdogs, serial, PCI, gpio hogs, etc.)

In short, the DM does lazy initialization of devices to boot quickly and 
waste few resources.

> The goal of course being to minimize the amount of probed devices.

This was never the goal. The probe() is called when needed, that's all.

> This of course requires having
> some things at the "root" of the tree. In Linux, everything bound is 
> probed until nothing more can
> be probed. In that sense, under Linux everything is the "root" of the 
> tree. But for U-Boot the
> second two options specify the "roots."
> 
> Some of the things in init_sequence are ad-hoc, especially the non-DM 
> stuff. LEDs are pretty ad-hoc,
> last I checked, but that's mainly because there are 2-3 separate non-DM 
> ways to control LEDs.

I'm not sure I understand this part, but maybe see

doc/develop/driver-model/design.rst

that should clarify the design decisions behind the DM.

>> but that's all there because there is no better way to implement that 
>> within the DM framework so far. Now there is, with this flag, so those 
>> workarounds can also be cleaned up.
> 
> This *is* within the DM framework.

No, init_sequence_r/init_sequence_f is U-Boot init sequence. The DM init 
is only one step in that init sequence. Please do not conflate those two 
concepts.

> Adding a flag which is set on bound 
> devices has the same effect
> as dm_mux_init does when called from initr_dm_devices.

This cannot scale, you would have to add a huge list of such functions 
to init_sequence_r/init_sequence_f probably for each uclass, which would 
each do some sort of ad-hoc device_probe(), outside of DM. And neither 
of those functions would have easy access to the context of each driver 
instance, so they would have to work around that somehow (that's usually 
done by wasting cycles on traversing DT again or other such heuristics).

> The only 
> difference is implementation. So
> why is this better?

See above. Also, this implementation is within the context of DM, i.e. 
driver/uclass bind call sets this flag for driver instances which for 
some reason need to be probe()d. The reason can be inferred from the 
driver instance context, e.g. because the DT node matching the driver 
instance contains some DT property, this flag is set, and the DM core 
probes the driver instead of only binding it. No need for any ad-hoc 
workarounds in init code.