On Wed 25 Aug 08:22 PDT 2021, Felipe Balbi wrote: > > Hi, > > Bjorn Andersson <bjorn.andersson@xxxxxxxxxx> writes: > >> Bjorn Andersson <bjorn.andersson@xxxxxxxxxx> writes: > >> > On Wed 07 Jul 20:06 PDT 2021, Peter Chen wrote: > >> > > >> >> 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: > >> >> > > >> >> > > 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
