On 21-07-07 14:03:19, Bjorn Andersson wrote:

On Tue 06 Jul 20:57 CDT 2021, Peter Chen wrote:

Allow me to reorder your two questions:

quoted

And why using a notifier need to concern core's deferral probe?

The problem at hand calls for the core for somehow invoking
dwc3_qcom_vbus_overrride_enable() with a pointer to dwc3_qcom passed.

This means that dwc3-qcom somehow needs to inform the dwc3-core about
this (and stash the pointer). And this can't be done until dwc3-core
actually exist, which it won't until dwc3_probe() has completed
successfully (or in particular allocated struct dwc).

Maybe you misunderstood the notifier I meant previous, my pointer was
calling glue layer API directly.

Role switch is from dwc3-core, when it occurs, it means structure dwc3 has
allocated successfully, you could call glue layer notifier at function
dwc3_usb_role_switch_set directly.
Some references of my idea [1] [2]

[1] Function ci_hdrc_msm_notify_event at ci_hdrc_msm_notify_event
[2] https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/usb/dwc3/core.c?h=lf-5.10.y#n205

quoted

Why do you think we need to retry the parent's probe again?

There's four options here:

0) Hope that of_platform_populate() always succeeds in invoking
dwc3_probe() on the first attempt, so that it is available when
of_find_device_by_node() is invoked in dwc3_qcom_probe() (and a few of
the other platform's drivers).

1) Ensure that the operations performed by dwc3_probe() happens
synchronously and return a failure to dwc3-qcom, which depending on how
dwc3_probe() failed can propagate that failure - i.e. either probe defer
or clean up its resources if the failure from dwc3-core is permanent.

2) Register the dwc3-core and then return from dwc3_qcom_probe() in some
half-initialized state and through some yet to be invented notification
mechanism continue the tail of dwc3_qcom_probe() once dwc3_probe() has
finished. But I know of no such notification mechanism in place today
and we can just register a callback, because of 1).
Furthermore we'd leave dwc3-qcom half-initialized until the dwc3-core is
done probing - which might never happen.

3) Make drvdata of all the platform drivers be some known struct that
dwc3-core can retrieve and dereference - containing the optional
callback to the role_switch callback.


We've tried the option 0) for a few years now. Option 2) is a variant of
what we have today, where we patch one problem up and hope that nothing
unexpected happens until things has fully probed. We're doing 3) in
various other places, but in my view it's abusing a void * and has to be
kept synchronized between all the possible parent drivers.

Left is 1), which will take some refactoring, but will leave the
interaction between the two drivers in a state that's very easy to
reason about.

Function of_find_device_by_node() invoked at glue layer is usually successfully,
The dwc3_probe failure doesn't affect it, unless you enable auto-suspend,
and glue layer's runtime suspend routine depends on dwc3 core's runtime suspend
routine. Would you please describe more about dwc3-core probe failure causes
dwc3-qcom's probe has failed or in half-initialized state you said?

quoted

I know there are some downstream code which using this way, I would
like to know the shortcoming for it.

The shortcoming of having dwc3_qcom_probe() invoke dwc3_probe()
"manually" and then returning -EPROBE_DEFER if the dwc3-core's resources
aren't yet available is that we're wasting some time tearing down the
dwc3-qcom state and then re-initialize it next time this is attempted.

Like above, would you explain more about it?

-- 

Thanks,
Peter Chen