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

[Felipe Balbi <balbi@kernel.org>]

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?

-- 
balbi