Re: [PATCH 0/3] Implement role-switch notifications from dwc3-drd to dwc3-qcom

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
	Felipe Balbi <balbi@kernel.org>,
	Peter Chen <peter.chen@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	agross@kernel.org, gregkh@linuxfoundation.org,
	jackp@codeaurora.org, wcheng@codeaurora.org,
	linux-usb@vger.kernel.org, linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH 0/3] Implement role-switch notifications from dwc3-drd to dwc3-qcom
Date: Wed, 25 Aug 2021 19:43:45 +0300	[thread overview]
Message-ID: <YSZzQXzrry+DFU6X@kuha.fi.intel.com> (raw)
In-Reply-To: <YSZned9v1+ajzVx0@ripper>

On Wed, Aug 25, 2021 at 08:53:29AM -0700, Bjorn Andersson wrote:
> On Wed 25 Aug 01:18 PDT 2021, Bryan O'Donoghue wrote:
> 
> > On 25/08/2021 06:51, Felipe Balbi wrote:
> > > > Hi Peter, I took a proper look at this again, hoping to find a way to
> > > > pass a callback pointer from dwc3-qcom to the dwc3 core, that can be
> > > > called from __dwc3_set_mode() to inform the Qualcomm glue about mode
> > > > changes.
> > > I would rather keep the strict separation between glue and core.
> > 
> > # 1 __dwc3_set_mode
> > Felipe wants to keep a strict separation between core and glue
> > 
> > # notifier
> > Requires the core probe() to complete before the glue probe to work
> > reliably. This then would lead us to remaking the dwc3-qcom::probe() to
> > facilitate probe deferral.
> > 
> > We can be sure bugs would be introduced in this process.
> > 
> > AFAIK Felipe is not opposed to this, Bjorn likes it

Notifiers were proposed for the USB role switches already some time
ago [1], and I don't think anybody was against them, but in the end I
don't think there were any users for those notifier, so they were
never added.

If something needs to only react to the role changes like I think in
this case, then I would just add those notifiers to the USB role
switches.

[1] https://lore.kernel.org/linux-usb/20191002231617.3670-3-john.stultz@linaro.org/

> Using a notifier or just a direct callback from core to the glue is an
> implementation detail, but as you say we need a way for the glue to
> register this before the core is fully probed.

There was an idea to add bind and unbind callbacks to the software
nodes (callbacks that would be called when a device is bind to the
node) at one point in order to solve this kind of problems.

In this case it would work so that you would supply a software node
for the role switch in your glue driver (that part is not a problem),
and then if the bind of that software node is called, you know the
role switch was registered, and you can acquire the handle to it
safely and start listening notifications from it.

But I don't know if that's a very sophisticated solution.

thanks,

-- 
heikki