> -----Original Message----- > From: Felipe Balbi <balbi@xxxxxxxxxx> > Sent: Monday, August 2, 2021 3:49 PM > To: Jun Li <jun.li@xxxxxxx> > Cc: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>; > gregkh@xxxxxxxxxxxxxxxxxxx; shawnguo@xxxxxxxxxx; dl-linux-imx > <linux-imx@xxxxxxx>; thunder.leizhen@xxxxxxxxxx; > linux-usb@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] usb: dwc3: imx8mp: detect dwc3 core node via compatible > string > > > Hi, > > Jun Li <jun.li@xxxxxxx> writes: > >> Jun Li <jun.li@xxxxxxx> 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