Re: [PATCH] media: venus: Synchronize probe() between venus_core and enc/dec

[Tadeusz Struk <tadeusz.struk@linaro.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1754 bytes --]

On 10/25/21 09:03, Bryan O'Donoghue wrote:
> I don't think there's any guarantee at all, that core probe() has completed at 
> that point.

I think there is, thanks to the new sync_mutex. The enc/dec probe will keep
returning -EPROBE_DEFER; until the core driver calls
platform_set_drvdata(pdev, core); in line 338, but before it does that it
takes the syn_lock. Then both enc/dec drivers will block on the same sync_lock
until either the core has finished initialization fully and unlocks it in line
378 just before returning 0, or it fails in between and unlocks it on the err
path. Only then the other two can proceed and check if the core probe failed,
in which case the condition core->state != CORE_INIT will be true.

> 
> of_platform_populate() doesn't guarantee ordering of the probe() completing 
> before or after the probe() of the platform drivers that are associated with the 
> devices in of_platform_populate().

agree, but I don't depend on of_platform_populate(). The ordering between the
three probe functions is enforced by the new sync mutex.

> 
> When you think it about it can't do that and you wouldn't want it to do that 
> since a device might have a legitimate reason to EPROBE_DEFER
> 
> As an example core could call of_platform_populate() and then as a ridiculous 
> example go to sleep for five seconds - in which case it is perfectly possible 

and this is exactly what happens when the core probe() loads the firmware from
disk.

> the encoder and decoder probe() functions will bug out illegitimately waiting 
> because of core->state != CORE_INIT

not really, because it will block on the mutex and only check the condition
after the sync_lock is unlocked.

-- 
Thanks,
Tadeusz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]