From patchwork Thu May 26 15:23:30 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thierry Reding X-Patchwork-Id: 626734 X-Patchwork-Delegate: treding@nvidia.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3rFtGW3Brzz9t62 for ; Fri, 27 May 2016 01:24:03 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=sgtqkzPL; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753971AbcEZPXm (ORCPT ); Thu, 26 May 2016 11:23:42 -0400 Received: from mail-pa0-f66.google.com ([209.85.220.66]:36651 "EHLO mail-pa0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753962AbcEZPXk (ORCPT ); Thu, 26 May 2016 11:23:40 -0400 Received: by mail-pa0-f66.google.com with SMTP id fg1so9371707pad.3; Thu, 26 May 2016 08:23:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=NFI5Lttt14RFroMNuAA0xkDP+pgryOc2Nmi+YM5FuBw=; b=sgtqkzPLrE42QL0k5j9NSo7COxGd9o3z3gPfQYpfLzU1ir/wCpIzIo1fBAKHmDjv5M ccczWx5x8c+CzBB8nmbUHGKyv4aYML/p3gma+XVgCOPFiM8jLw9le2Tj3ifF3rYK4hro PCJ4SHt1HK+q98qM8xhmp27g3dUQtEbB2k/mCdRDBwcg7vgTk6pBVd3KT4p53+umdRjE Xrv7ZZpJiPX3uV/3geaOKaPMjHfOeWdGacWvU+k7nKN+FBF2kY0DnZYFZHtwjQ7DF3ko 4oE5Au3IMVsir5qtGFbU1ZT+nA66O/gEHRdvYPM0kQT/CcuYkiXbG8PXyl3AVfVjKQRJ BQaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=NFI5Lttt14RFroMNuAA0xkDP+pgryOc2Nmi+YM5FuBw=; b=bnU0vk9HGwDkF8rmHbOP1cvZx72rjFgx0IXjaVWGpriFQXw9NXWiKLTA1hNVXH+dB3 q+2srAMeA3BvNlujfQbcAXZXBGrq8eL54go9DxTIMQA1fwSWVR76dE8OhblAao1fT5Vs 7ex7/R0r+TTWMIaeji1CBwOXs7RyKfbZkqIbBISnhy8XDW8djfD72owjx5hskWeXcuTS AavGUl1uOi61m68a50+0qGFCB1wOFQ2JU8TZOsS8Im2hIxWV98Q7HqmnIqvaAosVdnfV oEgWE959Np8f4pW+avy87Ct7BEtj7AAJC/VxDfBRwcP2IK2Ww/mLwZq1PHEFX2XQVtSW CuWQ== X-Gm-Message-State: ALyK8tJsCQ9TRDnktKlEWcNh5+eNeMHlkDxFN4ObpF/9CpLf2bJRO20OHV85i1hMFsHlvg== X-Received: by 10.66.171.231 with SMTP id ax7mr14748876pac.104.1464276219085; Thu, 26 May 2016 08:23:39 -0700 (PDT) Received: from localhost (port-54928.pppoe.wtnet.de. [46.59.215.64]) by smtp.gmail.com with ESMTPSA id f9sm6908591pfd.56.2016.05.26.08.23.37 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 26 May 2016 08:23:37 -0700 (PDT) From: Thierry Reding To: Alan Stern , Greg Kroah-Hartman Cc: Stephen Warren , Thierry Reding , Alexandre Courbot , Jon Hunter , linux-usb@vger.kernel.org, linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org, Philipp Zabel , Hans de Goede Subject: [PATCH v4 2/2] usb: host: ehci-tegra: Avoid getting the same reset twice Date: Thu, 26 May 2016 17:23:30 +0200 Message-Id: <20160526152330.10929-2-thierry.reding@gmail.com> X-Mailer: git-send-email 2.8.3 In-Reply-To: <20160526152330.10929-1-thierry.reding@gmail.com> References: <20160526152330.10929-1-thierry.reding@gmail.com> Sender: linux-tegra-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-tegra@vger.kernel.org From: Thierry Reding Starting with commit 0b52297f2288 ("reset: Add support for shared reset controls") there is a reference count for reset control assertions. The goal is to allow resets to be shared by multiple devices and an assert will take effect only when all instances have asserted the reset. In order to preserve backwards-compatibility, all reset controls become exclusive by default. This is to ensure that reset_control_assert() can immediately assert in hardware. However, this new behaviour triggers the following warning in the EHCI driver for Tegra: [ 3.365019] ------------[ cut here ]------------ [ 3.369639] WARNING: CPU: 0 PID: 1 at drivers/reset/core.c:187 __of_reset_control_get+0x16c/0x23c [ 3.382151] Modules linked in: [ 3.385214] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.6.0-rc6-next-20160503 #140 [ 3.392769] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree) [ 3.399046] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [ 3.406787] [] (show_stack) from [] (dump_stack+0x90/0xa4) [ 3.414007] [] (dump_stack) from [] (__warn+0xe8/0x100) [ 3.420964] [] (__warn) from [] (warn_slowpath_null+0x20/0x28) [ 3.428525] [] (warn_slowpath_null) from [] (__of_reset_control_get+0x16c/0x23c) [ 3.437648] [] (__of_reset_control_get) from [] (tegra_ehci_probe+0x394/0x518) [ 3.446600] [] (tegra_ehci_probe) from [] (platform_drv_probe+0x4c/0xb0) [ 3.455029] [] (platform_drv_probe) from [] (driver_probe_device+0x1ec/0x330) [ 3.463892] [] (driver_probe_device) from [] (__driver_attach+0xb8/0xbc) [ 3.472320] [] (__driver_attach) from [] (bus_for_each_dev+0x68/0x9c) [ 3.480489] [] (bus_for_each_dev) from [] (bus_add_driver+0x1a0/0x218) [ 3.488743] [] (bus_add_driver) from [] (driver_register+0x78/0xf8) [ 3.496738] [] (driver_register) from [] (do_one_initcall+0x40/0x170) [ 3.504909] [] (do_one_initcall) from [] (kernel_init_freeable+0x158/0x1f8) [ 3.513600] [] (kernel_init_freeable) from [] (kernel_init+0x8/0x114) [ 3.521770] [] (kernel_init) from [] (ret_from_fork+0x14/0x3c) [ 3.529361] ---[ end trace 4bda87dbe4ecef8a ]--- The reason is that Tegra SoCs have three EHCI controllers, each with a separate reset line. However the first controller contains UTMI pads configuration registers that are shared with its siblings and that are reset as part of the first controller's reset. There is special code in the driver to assert and deassert this shared reset at probe time, and it does so irrespective of which controller is probed first to ensure that these shared registers are reset before any of the controllers are initialized. Unfortunately this means that if the first controller gets probed first, it will request its own reset line and will subsequently request the same reset line again (temporarily) to perform the reset. This used to work fine before the above-mentioned commit, but now triggers the new WARN. Work around this by making sure we reuse the controller's reset if the controller happens to be the first controller. Cc: Philipp Zabel Cc: Hans de Goede Reviewed-by: Philipp Zabel Signed-off-by: Thierry Reding --- Changes in v4: - avoid calling reset_control_put() on ERR_PTR()-encoded error codes Changes in v3: - reword commit message to more accurately describe the hardware design Changes in v2: - restore has_utmi_pad_registers condition (Alan Stern) drivers/usb/host/ehci-tegra.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c index c1c1024a054c..9a3d7db5be57 100644 --- a/drivers/usb/host/ehci-tegra.c +++ b/drivers/usb/host/ehci-tegra.c @@ -81,15 +81,23 @@ static int tegra_reset_usb_controller(struct platform_device *pdev) struct usb_hcd *hcd = platform_get_drvdata(pdev); struct tegra_ehci_hcd *tegra = (struct tegra_ehci_hcd *)hcd_to_ehci(hcd)->priv; + bool has_utmi_pad_registers = false; phy_np = of_parse_phandle(pdev->dev.of_node, "nvidia,phy", 0); if (!phy_np) return -ENOENT; + if (of_property_read_bool(phy_np, "nvidia,has-utmi-pad-registers")) + has_utmi_pad_registers = true; + if (!usb1_reset_attempted) { struct reset_control *usb1_reset; - usb1_reset = of_reset_control_get(phy_np, "utmi-pads"); + if (!has_utmi_pad_registers) + usb1_reset = of_reset_control_get(phy_np, "utmi-pads"); + else + usb1_reset = tegra->rst; + if (IS_ERR(usb1_reset)) { dev_warn(&pdev->dev, "can't get utmi-pads reset from the PHY\n"); @@ -99,13 +107,15 @@ static int tegra_reset_usb_controller(struct platform_device *pdev) reset_control_assert(usb1_reset); udelay(1); reset_control_deassert(usb1_reset); + + if (!has_utmi_pad_registers) + reset_control_put(usb1_reset); } - reset_control_put(usb1_reset); usb1_reset_attempted = true; } - if (!of_property_read_bool(phy_np, "nvidia,has-utmi-pad-registers")) { + if (!has_utmi_pad_registers) { reset_control_assert(tegra->rst); udelay(1); reset_control_deassert(tegra->rst);