From patchwork Mon May 28 15:41:48 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Johan Hovold X-Patchwork-Id: 10433445 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id B7936602CC for ; Mon, 28 May 2018 15:42:11 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A54E62899E for ; Mon, 28 May 2018 15:42:11 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9969928AF1; Mon, 28 May 2018 15:42:11 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 361932899E for ; Mon, 28 May 2018 15:42:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1034744AbeE1Pl7 (ORCPT ); Mon, 28 May 2018 11:41:59 -0400 Received: from mail-wr0-f195.google.com ([209.85.128.195]:44592 "EHLO mail-wr0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965409AbeE1Plv (ORCPT ); Mon, 28 May 2018 11:41:51 -0400 Received: by mail-wr0-f195.google.com with SMTP id y15-v6so20937049wrg.11; Mon, 28 May 2018 08:41:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=LsGkiJOzfRmyKTDdOpnZOLM9Pqq+hFlvFvUr02C4Jk0=; b=bgpKLYNPDQ2tFaEB0GJOldhk+0eUme8fcLU2STyKBc0vvMbT4tidMiP+BrqnUlUTqi n3nSOvvEBBoVYCdVE4BzkfBp5xLqeIrqQjDLjCOCal85OSXclSpep71VBfabEDLDirDX 1c4XoNx9XJSHsk76O5WZ1Q1WR5EAcZZ9jNOJ5zY5QehSlxUIe8j8FEPIfKo3hXb/qGaN MXXgMIQTUVDnSoJerWeppcLlO1Xm+x/rYEF1+0AZvmJHCH+eXBiAzH7mL/9qFrk7METH J1G79WukNUZ93ux2o8RmPHguGJ3whXbCWOolFSoUf4op6hnAiBwXUChyaRR4aoPEmrQQ b+3w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to:user-agent; bh=LsGkiJOzfRmyKTDdOpnZOLM9Pqq+hFlvFvUr02C4Jk0=; b=YAHUe0aomfgKPqQbzcOtO7WXcrPHOrPFol9wEiVeAU4UrhLZIMgZZl1pTyePpACgqs 1A/P4kW0cPlXXIIqlV7IQGfQhcIM+/3akBGFZWGzx22rxhyrgPP+spwVozz1OVoyLIlr TfQS+A0lUYK9QRuWRJQicpVoBq9h71UftmZyDSfc5T1uz22jlFiIwSbIwF7Kb2+5LOOU seLetNnjEUjR3wnvva6ygdeuB4BZPcAbMGjK8UkUeGXcnvenRS2TzxWO3OH8T5P2/PF6 BhmTochXPSQebRUzC3Qy0NB7lTqlzBs6wJe4/WcLzm6x82bY5QqouNDmIay2jrAsicOs elOw== X-Gm-Message-State: ALKqPwceeqYJPgDDLBf62egygYW4xphDwHmjo3U3IOIg8ygSvngoG5bA GFO7xynfhaW0D5nJKunyULo= X-Google-Smtp-Source: ADUXVKIRrqsITqC+Ba6c5djv7RRoOaCaH+DqUYNpElz1xgJLt5thwTo9UatL9fDJXqem4yXVCGqj1g== X-Received: by 2002:a19:7:: with SMTP id 7-v6mr1280013lfa.62.1527522110160; Mon, 28 May 2018 08:41:50 -0700 (PDT) Received: from xi.terra (c-8bb2e655.07-184-6d6c6d4.bbcust.telenor.se. [85.230.178.139]) by smtp.gmail.com with ESMTPSA id u8-v6sm6013045ljg.40.2018.05.28.08.41.49 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 28 May 2018 08:41:49 -0700 (PDT) Received: from johan by xi.terra with local (Exim 4.90_1) (envelope-from ) id 1fNKHM-0002G3-Od; Mon, 28 May 2018 17:41:48 +0200 Date: Mon, 28 May 2018 17:41:48 +0200 From: Johan Hovold To: Roger Quadros Cc: balbi@kernel.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] usb: dwc3: of_simple: don't call pm_runtime_set_active() Message-ID: <20180528152030.GA3261@localhost> References: <1527518174-27860-1-git-send-email-rogerq@ti.com> <1527518174-27860-2-git-send-email-rogerq@ti.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1527518174-27860-2-git-send-email-rogerq@ti.com> User-Agent: Mutt/1.10.0 (2018-05-17) Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, May 28, 2018 at 05:36:14PM +0300, Roger Quadros wrote: > Don't call pm_runtime_set_active() as it will prevent the device > from being activated in the next pm_runtime_get_sync() call. > > Also call pm_runtime_get_sync() before of_platform_populate(). This paragraph describes what you do, but not why do it. > Signed-off-by: Roger Quadros > --- > drivers/usb/dwc3/dwc3-of-simple.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c > index e98d221..2cbb5c0 100644 > --- a/drivers/usb/dwc3/dwc3-of-simple.c > +++ b/drivers/usb/dwc3/dwc3-of-simple.c > @@ -121,6 +121,9 @@ static int dwc3_of_simple_probe(struct platform_device *pdev) > if (ret) > goto err_resetc_assert; > > + pm_runtime_enable(dev); > + pm_runtime_get_sync(dev); This breaks runtime pm as you now get a second round of clock enables which are never balanced on runtime suspend (the clocks are first enabled in dwc3_of_simple_clk_init() above and with your change again in dwc3_of_simple_runtime_resume()). On the other hand, we currently return from probe() with a positive RPM count so perhaps the RPM callbacks can just be removed altogether (i.e. unless some other entity drops that count at some point before remove()). > ret = of_platform_populate(np, NULL, NULL, dev); > if (ret) { > for (i = 0; i < simple->num_clocks; i++) { > @@ -131,10 +134,6 @@ static int dwc3_of_simple_probe(struct platform_device *pdev) > goto err_resetc_assert; > } > > - pm_runtime_set_active(dev); > - pm_runtime_enable(dev); > - pm_runtime_get_sync(dev); > - > return 0; > > err_resetc_assert: Also note that there's currently a use-after-free in remove(), where pm_runtime_put_sync() is called after the clocks have been put. Something like the below (untested) patch should fix it. Johan From 35c384c31010c344d403c26fc0a1dde0fd68ef4a Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 28 May 2018 17:31:45 +0200 Subject: [PATCH] usb: dwc3: of-simple: fix use-after-free on remove The clocks have already been explicitly disabled and put as part of remove() so the runtime suspend callback must not be run when balancing the runtime PM usage count before returning. Fixes: 16adc674d0d6 ("usb: dwc3: add generic OF glue layer") Signed-off-by: Johan Hovold --- drivers/usb/dwc3/dwc3-of-simple.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c index cb2ee96fd3e8..b9c869cd6585 100644 --- a/drivers/usb/dwc3/dwc3-of-simple.c +++ b/drivers/usb/dwc3/dwc3-of-simple.c @@ -165,8 +165,9 @@ static int dwc3_of_simple_remove(struct platform_device *pdev) reset_control_put(simple->resets); - pm_runtime_put_sync(dev); + pm_runtime_put_noidle(dev); pm_runtime_disable(dev); + pm_runtime_set_suspended(dev); return 0; }