Thread (36 messages) 36 messages, 7 authors, 2021-09-17

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

From: Bjorn Andersson <hidden>
Date: 2021-08-25 16:32:10
Also in: linux-arm-msm

On Wed 25 Aug 08:22 PDT 2021, Felipe Balbi wrote:
Hi,

Bjorn Andersson [off-list ref] writes:
quoted
quoted
Bjorn Andersson [off-list ref] writes:
quoted
On Wed 07 Jul 20:06 PDT 2021, Peter Chen wrote:
quoted
On 21-07-07 14:03:19, Bjorn Andersson wrote:
quoted
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
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.
I'm okay with that goal, but the result is that both the OMAP and
Qualcomm driver duplicates the extcon interface already present in the
DRD, and the Meson driver duplicates the usb_role_switch. In addition to
the code duplication this manifest itself in the need for us to link
extcon to both the glue and core nodes in DeviceTree.

In order to function in a USB-C based setup we now need to register a 
usb_role_switch from the Qualcomm glue and we need to evolve the
usb_role_switch implementation to allow for the Type-C controller to
notify more than a single role-switcher.

So we're facing the need to introduce another bunch of duplication and
the DT will be quite ugly with both glue and core having to set up an
of_graph with the Type-C controller.


I really would like for us to come up with a way where the core can
notify the glue that role switching is occurring, so that we instead of
adding more duplication could aim to, over time, remove the extcon and
usb_role_switch logic from the Qualcomm, OMAP and Meson drivers.
We can make a comparison between clk rate notifiers. Anyone can
subscribe to a clk rate notification and react to the notification. A
generic dual role notification system should allow for something
similar. I really don't get why folks want to treat a glue and core
driver differently in this case.

Why do we want to pass function pointers around instead of letting
whatever role notification mechanism to be able to notify more than one
user?

Also keep in mind that we have dwc3 implementations which are dual role
capable but don't ship the synopsys DRD block. Rather, there's a
peripheral-only dwc3 instance and a separate xhci with custom logic
handling role swap.
So you're suggesting that we invent a 3rd mechanism (in addition to the
already existing duplication between extcon and usb_role_switch) for
propagating role switching notifications through the kernel?
If we were to provide a dwc3-specific role swap function-pointer based
interface, we would just create yet another special case for this. A
better approach would be to start consolidating all of these different
role-swap mechanisms in a generic layer that "knows-it-all". If dwc3 is
generating the role notification or a separate type-c controller or even
some EC IRQ, that shouldn't matter for the listeners.
I was under the impression that usb_role_switch is the attempt to
replace extcon as the one solution. The problem in the dwc3 case is that
the same piece of hardware (i.e. _the_ USB controller) needs to
implement and wired up as two separate consumers of that message.

I recognize the complexity caused by the flexibility in DWC3 based
designs, but I would like to see whatever combination be seen as a
single entity to the rest of the system - rather than building the
notification plumbing between the two pieces using DeviceTree.

Regards,
Bjorn
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help