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

[Jun Li <jun.li@nxp.com>]

Hi,
> -----Original Message-----
> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Sent: Tuesday, May 11, 2021 3:59 PM
> To: Jun Li <jun.li@nxp.com>
> Cc: Felipe Balbi <balbi@kernel.org>; gregkh@linuxfoundation.org;
> 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 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.

So I had the incorrect impression.

Yes, that's the Felipe was referring to, I think.

I will pick up [1] and improve my driver as Felipe suggested.

Thanks
Li Jun

> 
> Or, are we talking about some other notifier here?
> 
> [1]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
> kernel.org%2Flinux-usb%2F20191002231617.3670-3-john.stultz%40linaro.org
> %2F&amp;data=04%7C01%7Cjun.li%40nxp.com%7C9479f8f61e1a47245c2e08d91452a
> dd6%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637563167667138204%7CU
> nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haW
> wiLCJXVCI6Mn0%3D%7C1000&amp;sdata=EhSPBC1sGeH65EPuFV22pEcVS6nDeRNPdN9Z1
> 3lPPYg%3D&amp;reserved=0
> 
> 
> thanks,
> 
> --
> heikki