Date:   Thu, 30 Jun 2022 18:10:50 -0700
From:   Matthias Kaehlcke <mka@...omium.org>
To:     Krishna Kurapati PSSNV <quic_kriskura@...cinc.com>
Cc:     Stephen Boyd <swboyd@...omium.org>,
        Pavan Kondeti <quic_pkondeti@...cinc.com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Felipe Balbi <balbi@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Andy Gross <agross@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Doug Anderson <dianders@...omium.org>,
        Mathias Nyman <mathias.nyman@...el.com>,
        devicetree@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-pm@...r.kernel.org, quic_ppratap@...cinc.com,
        quic_vpulyala@...cinc.com
Subject: Re: [PATCH v20 2/5] usb: dwc3: core: Host wake up support from
 system suspend

On Thu, Jun 30, 2022 at 11:43:01PM +0530, Krishna Kurapati PSSNV wrote:
> 
> On 6/30/2022 3:45 AM, Stephen Boyd wrote:
> > Quoting Pavan Kondeti (2022-06-27 22:31:48)
> > > On Mon, Jun 27, 2022 at 01:02:49PM -0700, Stephen Boyd wrote:
> > > > Quoting Pavan Kondeti (2022-06-20 01:54:15)
> > > > > Would like to hear other people thoughts on this.
> > > > > 
> > > > I'm not following very closely but it sounds like a problem that may be
> > > > solved by using the component driver code (see
> > > > include/linux/component.h). That would let you move anything that needs
> > > > to be done once the child devices probe to the aggregate driver 'bind'
> > > > function (see struct component_master_ops::bind).
> > > Thanks Stephen for letting us know about the component device framework.
> > > 
> > > IIUC,
> > > 
> > > - dwc3-qcom (parent of the dwc3 core) registers as a component master by
> > > calling component_master_add_with_match() before calling
> > > of_platform_populate(). The match callback could be as simple as comparing
> > > the device against our child device.
> > > 
> > > - The dwc3 core (child) at the end of its probe can add as a component by calling
> > > component_add().
> > > 
> > > - The above triggers the component_master_ops::bind callback implemented in
> > >    dwc3-qcom driver which signals that we are good to go.
> > > 
> > > - The dwc-qcom can call component_bind_all() to finish the formality i.e
> > >    telling the dwc3 core that we are good to go.
> > > 
> > > Is my understanding correct? This is what we are looking for i.e a way for
> > > the child device(s) to signal the parent when the former is bounded.
> > Sounds about right to me.
> > 
> > > Also what happens when the child device probe fails for any reason. i.e
> > > component_add() would never be called so the master driver i.e dwc3-qcom would
> > > wait indefinitely. May be it needs to implement a timeout or runtime suspend
> > > etc should take care of keeping the resoures in suspend state.
> > When the child fails probe, it should return -EPROBE_DEFER if probe
> > needs to be deferred. Then the driver will attempt probe at a later
> > time. If probe fails without defer then it will never work and dwc3-qcom
> > will wait indefinitely. Not much we can do in that situation.
> Hi Stephen,
> 
>   Thanks for the idea. But doesn't adding dwc3 as a component to an agg
> driver meanthat this change needs to be done on all glue drivers, as
> component_bind_all( ) from master componentis supposed to let the dwc3
> core know that we are good to go ?

Ideally all glue drivers would add component support, however I don't think
it is strictly necessary. Currently the dwc3 core already assumes that
everything is in place when it is probed. The core could have empty bind()
and unbind() callbacks, with that things in the core would remain
essentially as they are and the core doesn't depend on the glue driver to
call component_bind_all().

> > dwc3-qcom should wait for dwc3 core to call component_add() and then do
> > whatever needs to be done once the dwc3 core is registered in the
> > dwc3-qcom bind callback. Honestly this may all be a little overkill if
> > there's only two drivers here, dwc3-qcom and dwc3 core. It could
> > probably just be some callback from dwc3 core at the end of probe that
> > calls some function in dwc3-qcom.
> Since the issue we are facing is that the ssphy device links are not ready
> causing the dwc3 probe not being invoked, can we add an API as Pavan
> suggested
> to check if deferred_probe listfor dwc3 device is empty or not andbased on
> that we can choose to defer our qcomprobe ? In this case, we don't need to
> touch the dwc3 core driver and would be making changesonly in qcom glue
> driver.

As mentioned above, it shouldn't be necessary to add component support to
all the glue drivers. An API to check for deferred probing is an option,
however there is a possible race condition: When the dwc3-qcom driver checks
for a deferred probe the core could still be probing, in that situation the
glue would proceed before the core driver is ready. That could be avoided
with the component based approach.

Powered by blists - more mailing lists