Re: [PATCH] usb: dwc3: imx8mp: detect dwc3 core node via compatible string

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

Hi guys,

On Fri, May 07, 2021 at 07:31:43AM +0000, Jun Li wrote:
> Hi
> 
> > -----Original Message-----
> > From: Felipe Balbi <balbi@kernel.org>
> > Sent: Thursday, May 6, 2021 10:32 PM
> > To: Jun Li <jun.li@nxp.com>; gregkh@linuxfoundation.org
> > Cc: shawnguo@kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> > thunder.leizhen@huawei.com; linux-usb@vger.kernel.org
> > Subject: RE: [PATCH] usb: dwc3: imx8mp: detect dwc3 core node via compatible
> > string
> > 
> > 
> > Hi,
> > 
> > Jun Li <jun.li@nxp.com> writes:
> > >> > 	val = readl(dwc3_imx->glue_base + USB_WAKEUP_CTRL);
> > >> >
> > >> > 	if ((dwc3->current_dr_role == DWC3_GCTL_PRTCAP_HOST) && dwc3->xhci)
> > >> > 		val |= USB_WAKEUP_EN | USB_WAKEUP_SS_CONN |
> > >> > 		       USB_WAKEUP_U3_EN | USB_WAKEUP_DPDM_EN;
> > >> > 	else if (dwc3->current_dr_role == DWC3_GCTL_PRTCAP_DEVICE)
> > >> > 		val |= USB_WAKEUP_EN | USB_WAKEUP_VBUS_EN |
> > >> > 		       USB_WAKEUP_VBUS_SRC_SESS_VAL;
> > >>
> > >> for this, you could register a listener to the extcon notifier and
> > >> update these bits accordingly. With that, you would already *know*
> > >> that
> > >> dwc3 is probed.
> > >
> > > With usb role switch class, there no extcon provider, so I think this
> > > way can't work for me.
> > 
> > perhaps role switch class could learn about notifiers ;-)
> > 
> > >> > static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx) {
> > >> > 	struct dwc3_imx8mp	*dwc3_imx = _dwc3_imx;
> > >> > 	struct dwc3		*dwc = platform_get_drvdata(dwc3_imx->dwc3);
> > >> >
> > >> > 	if (!dwc3_imx->pm_suspended)
> > >> > 		return IRQ_HANDLED;
> > >> >
> > >> > 	disable_irq_nosync(dwc3_imx->irq);
> > >> > 	dwc3_imx->wakeup_pending = true;
> > >> >
> > >> > 	if ((dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST) && dwc->xhci)
> > >> > 		pm_runtime_resume(&dwc->xhci->dev);
> > >> > 	else if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_DEVICE)
> > >> > 		pm_runtime_get(dwc->dev);
> > >> >
> > >> > 	return IRQ_HANDLED;
> > >> > }
> > >>
> > >> for this, maybe you need to teach dwc3 core about wakeup irqs
> > >> instead. Have a look dev_pm_set_dedicated_wake_irq().
> > >
> > > Good suggestion, but if extcon notifier listener can't work for me, my
> > > understanding is this *teach* in glue layer driver still need access
> > > dwc3 core instance struct, right?
> > 
> > for now, maybe. But it may be better to implement a notifier method in role
> > switch class.
> 
> I am not sure if introduce notifier in role switch class is a good idea,
> I had the impression extcon is not encouraged to use if possible.

I'm not against role switch notifiers. They were proposed before
already couple of years ago, but at that time there just were no users
them notifier, so the patch was just dropped from the series [1].
But I don't think anybody was against the idea. Please feel free to
add them to the class if you have use for them.

Or, are we talking about some other notifier here?

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


thanks,

-- 
heikki