Re: [PATCH] usb: dwc2: Fix shutdown callback in platform

[Doug Anderson <dianders@chromium.org>]

Hi,

On Fri, May 29, 2020 at 4:51 AM Minas Harutyunyan
<Minas.Harutyunyan@synopsys.com> wrote:
>
> To avoid lot of interrupts from dwc2 core, which can be asserted in
> specific conditions need to disable interrupts on HW level instead of
> disable IRQs on Kernel level, because of IRQ can be shared between
> drivers.
>
> Cc: stable@vger.kernel.org
> Fixes: a40a00318c7fc ("usb: dwc2: add shutdown callback to platform variant")
> Tested-by: Frank Mori Hess <fmh6jj@gmail.com>
> Signed-off-by: Minas Harutyunyan <hminas@synopsys.com>
> ---
>  drivers/usb/dwc2/platform.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index e571c8ae65ec..ada5b66b948e 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -342,7 +342,7 @@ static void dwc2_driver_shutdown(struct platform_device *dev)
>  {
>         struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);
>
> -       disable_irq(hsotg->irq);
> +       dwc2_disable_global_interrupts(hsotg);
>  }

I could be wrong, but I think it would be better to instead end up
with both calls, like:

dwc2_disable_global_interrupts(hsotg);
disable_irq(hsotg->irq);

To some extent it's slightly overkill, but the disable_irq() function
has the nice "and wait for completion" bit.  Your new call doesn't do
this.

That being said, though, you still won't wait for the completion of
the IRQ handler for the "other drivers" you reference, right.  Maybe a
better fix would be to add a shutdown callback for those other drivers
and just keep relying on disable_irq()?


-Doug