Date: Mon, 24 Apr 2017 11:51:09 +0800
From: Peter Chen <peter.chen@nxp.com>
To: Bernhard Walle <bernhard@bwalle.de>
CC: <linux-usb@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] usb: chipidea: Fix missing resume call after suspend
Message-ID: <20170424035109.GA11436@b29397-desktop>
References: <20170421101053.2964-1-bernhard@bwalle.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20170421101053.2964-1-bernhard@bwalle.de>
User-Agent: Mutt/1.5.24 (2015-08-30)
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Apr 2017 03:51:19.4907
 (UTC)
X-MS-Exchange-CrossTenant-Id: 5afe0b00-7697-4969-b663-5eab37d5f47e
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=5afe0b00-7697-4969-b663-5eab37d5f47e;Ip=[192.88.168.50];Helo=[tx30smr01.am.freescale.net]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY1PR0301MB1189
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1871
Lines: 47

On Fri, Apr 21, 2017 at 12:10:53PM +0200, Bernhard Walle wrote:
> We have a i.MX53-based hardware (quite similar to the i.MX53 QSB from
> Freescale/NXP). I'm reading the ..../ci_hdrc.0/gadget/suspended sysfs
> file to find out whether a PC is connected to the USB gadget. With old
> kernel versions, this worked. However, with kernel 4.9 this didn't work.
> 
> When the host is suspended once, it never sets back the suspended status
> to 0. The reason is that this seems to be done in the resume handler,
> which should be executed in the interrupt handler:
> 
> udc_irq:
> 
> ...
>      if (USBi_PCI & intr) {
>              ci->gadget.speed = hw_port_is_high_speed(ci) ?
>                      USB_SPEED_HIGH : USB_SPEED_FULL;
>              if (ci->suspended && ci->driver->resume) {
>                      spin_unlock(&ci->lock);
>                      ci->driver->resume(&ci->gadget);
>                      spin_lock(&ci->lock);
>                      ci->suspended = 0;
>              }
>      }
> ...
> 
> However, ci->suspended is already 0 here because _gadget_stop_activity
> is called before. So the resume handler never gets called. The obvious
> solution is to not touch ci->suspended in _gadget_stop_activity and
> to trust the interrupt handler to set it back (and to modify it to set
> ci->suspended to 0 even if ci->driver->resume is NULL).

The current code logic is:
- When the resume is received from host, the ci->dirver->resume is
called, and suspended is cleared.
- When the reset is received from host, the isr_reset_handler is called,
and suspended is cleared by _gadget_stop_activity. Since reset is
called, so ci->driver->resume doesn't need to be called.

There is a patch to fix clear suspended even the ci->driver->resume is
NULL at v4.12-rc1.

      usb: chipidea: udc: update gadget state after bus resume

-- 

Best Regards,
Peter Chen