Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v5] usb: ohci: disable start-of-frame interrupt in
 ohci_rh_suspend
To:     Alan Stern <stern@rowland.harvard.edu>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
        Greg Kroah-Hartman <greg@kroah.com>,
        Patchwork Bot <patchwork-bot@kernel.org>
References: <1634027450-3358-1-git-send-email-zhuyinbo@loongson.cn>
From:   zhuyinbo <zhuyinbo@loongson.cn>
Message-ID: <c26e86a7-a24d-42f5-3d39-71c34f8c5e31@loongson.cn>
Date:   Tue, 12 Oct 2021 20:11:45 +0800
User-Agent: Mozilla/5.0 (X11; Linux mips64; rv:68.0) Gecko/20100101
 Thunderbird/68.7.0
MIME-Version: 1.0
In-Reply-To: <1634027450-3358-1-git-send-email-zhuyinbo@loongson.cn>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Precedence: bulk


在 2021/10/12 下午4:30, Yinbo Zhu 写道:
> The usb states of ohci controller include UsbOperational, UsbReset,
> UsbSuspend and UsbResume. Among them, only the UsbOperational state
> supports launching the start-of-frame for host controller according
> the ohci protocol spec, but in S3 and S4 (suspend to memory/suspend
> to disk) press test procedure, it may happen that the start-of-
> frame was launched in UsbSuspend status and cause ohci works failed
> that the phenomenon was hc will allways reproduce the SoF interrupt
> and consider that hc doesn't deal with the ed/td/done list in non-
> UsbOperational, and this patch was to disable SoF interrupt in ohci
> _rh_suspend so that it can fix ohci SoF abnormally interrupt issue.
>
> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
> ---
> Change in v5:
> 		Move the key code change into ohci_rh_suspend.
> 		Rework the commit log information.
>
>
>   drivers/usb/host/ohci-hub.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c
> index f474f2f..80a0094 100644
> --- a/drivers/usb/host/ohci-hub.c
> +++ b/drivers/usb/host/ohci-hub.c
> @@ -88,6 +88,8 @@ static int ohci_rh_suspend (struct ohci_hcd *ohci, int autostop)
>   		msleep (8);
>   		spin_lock_irq (&ohci->lock);
>   	}
> +	/* All ED unlinks should be finished, no need for SOF interrupts */
> +	ohci_writel(ohci, OHCI_INTR_SF, &ohci->regs->intrdisable);
>   	update_done_list(ohci);
>   	ohci_work(ohci);

Hi Alan Stern,

     I consider that autostop sometimes was '1' then and HC doesn't  
stop deal with the ed/td/done list , then ohci_wok will called 
start_ed_unlink and

     start_ed_unlink will enable INTR_SF as follows, so I think add 
above two line code after ohci_work it is more appropriate, Do you think so?

       static void start_ed_unlink (struct ohci_hcd *ohci, struct ed *ed)
        {

              ...

              494         /* enable SOF interrupt */
              495         ohci_writel (ohci, OHCI_INTR_SF, 
&ohci->regs->intrstatus);
              496         ohci_writel (ohci, OHCI_INTR_SF, 
&ohci->regs->intrenable);

               ...

         }


BRs,

Yinbo Zhu.

>