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

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

> -----Original Message-----
> From: Felipe Balbi <balbi@kernel.org>
> Sent: Monday, August 2, 2021 3:49 PM
> To: Jun Li <jun.li@nxp.com>
> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>;
> 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,
> 
> Jun Li <jun.li@nxp.com> writes:
> >> Jun Li <jun.li@nxp.com> writes:
> >> >> > > > 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.
> >>
> >> sounds great, thahnks Li Jun
> >
> > Got chance to check this, but it turns out this glue driver still has
> > to be care of the dwc3 core and its probe completion, because the role
> > switch class is created by dwc3 core in its probe.
> >
> > dev_pm_set_dedicated_wake_irq() is a good solution for case a
> > sperate/glue driver is not required, in my imx8mp case, I need a glue
> > driver anyway, so this seems can't make my driver much simpler. Rough
> > change please see below:
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > index 9f7c7f587d38..bcb63bcf27be 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > @@ -874,7 +874,6 @@ usb3_0: usb@32f10100 {
> >  			clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
> >  				 <&clk IMX8MP_CLK_USB_ROOT>;
> >  			clock-names = "hsio", "suspend";
> > -			interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
> >  			#address-cells = <1>;
> >  			#size-cells = <1>;
> >  			dma-ranges = <0x40000000 0x40000000 0xc0000000>; @@ -891,7 +890,9
> > @@ usb_dwc3_0: usb@38100000 {
> >  				assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> >  				assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> >  				assigned-clock-rates = <500000000>;
> > -				interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
> > +				interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>,
> > +							<GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
> > +				interrupt-names = "irq", "wakeup";
> >  				phys = <&usb3_phy0>, <&usb3_phy0>;
> >  				phy-names = "usb2-phy", "usb3-phy";
> >  				snps,dis-u2-freeclk-exists-quirk; @@ -915,7 +916,6 @@
> usb3_1:
> > usb@32f10108 {
> >  			clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
> >  				 <&clk IMX8MP_CLK_USB_ROOT>;
> >  			clock-names = "hsio", "suspend";
> > -			interrupts = <GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>;
> >  			#address-cells = <1>;
> >  			#size-cells = <1>;
> >  			dma-ranges = <0x40000000 0x40000000 0xc0000000>; @@ -932,7 +932,9
> > @@ usb_dwc3_1: usb@38200000 {
> >  				assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> >  				assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> >  				assigned-clock-rates = <500000000>;
> > -				interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
> > +				interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>,
> > +							<GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>;
> > +				interrupt-names = "irq", "wakeup";
> >  				phys = <&usb3_phy1>, <&usb3_phy1>;
> >  				phy-names = "usb2-phy", "usb3-phy";
> >  				snps,dis-u2-freeclk-exists-quirk; diff --git
> > a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index
> > b6871ee5e8ca..4dac7cd98a31 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -1615,6 +1615,12 @@ static int dwc3_probe(struct platform_device *pdev)
> >  	if (ret)
> >  		return ret;
> >
> > +	dwc->wakeup_irq = platform_get_irq_byname_optional(pdev, "wakeup");
> > +	if (dwc->wakeup_irq == -EPROBE_DEFER)
> > +		goto assert_reset;
> > +	else
> > +		dev_err(dwc->dev, "the wakeup irq No. is %d\n", dwc->wakeup_irq);
> 
> why is this an error?

Forget to remove this debug purpose log, sorry.

Li Jun 

> 
> --
> balbi