From patchwork Tue Apr 12 14:06:16 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sudeep Holla X-Patchwork-Id: 65627 Delivered-To: patch@linaro.org Received: by 10.140.93.198 with SMTP id d64csp1941432qge; Tue, 12 Apr 2016 07:06:33 -0700 (PDT) X-Received: by 10.98.8.91 with SMTP id c88mr4880860pfd.47.1460469993447; Tue, 12 Apr 2016 07:06:33 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id tz5si10494378pab.239.2016.04.12.07.06.33; Tue, 12 Apr 2016 07:06:33 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934028AbcDLOGW (ORCPT + 29 others); Tue, 12 Apr 2016 10:06:22 -0400 Received: from foss.arm.com ([217.140.101.70]:55948 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932148AbcDLOGT (ORCPT ); Tue, 12 Apr 2016 10:06:19 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 50C8F28; Tue, 12 Apr 2016 07:05:05 -0700 (PDT) Received: from [10.1.207.150] (e103737-lin.cambridge.arm.com [10.1.207.150]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B84FB3F246; Tue, 12 Apr 2016 07:06:17 -0700 (PDT) From: Sudeep Holla Subject: Re: Unhandled fault during system suspend in sky2_shutdown To: Stephen Hemminger References: <570BCFC5.4070208@arm.com> <20160411112430.2a03e296@xeon-e3> Cc: Sudeep Holla , linux-kernel@vger.kernel.org, netdev@vger.kernel.org, Mirko Lindner Organization: ARM Message-ID: <570D00D8.7000008@arm.com> Date: Tue, 12 Apr 2016 15:06:16 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <20160411112430.2a03e296@xeon-e3> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/04/16 19:24, Stephen Hemminger wrote: > On Mon, 11 Apr 2016 17:24:37 +0100 > Sudeep Holla wrote: > [...] >> >> diff --git i/drivers/net/ethernet/marvell/sky2.c >> w/drivers/net/ethernet/marvell/sky2.c >> index ec0a22119e09..0ff0434e32fc 100644 >> --- i/drivers/net/ethernet/marvell/sky2.c >> +++ w/drivers/net/ethernet/marvell/sky2.c >> @@ -5220,6 +5220,13 @@ static SIMPLE_DEV_PM_OPS(sky2_pm_ops, >> sky2_suspend, sky2_resume); >> >> static void sky2_shutdown(struct pci_dev *pdev) >> { >> + struct sky2_hw *hw = pci_get_drvdata(pdev); >> + int i; >> + >> + for (i = hw->ports - 1; i >= 0; --i) { >> + sky2_detach(hw->dev[i]); >> + unregister_netdev(hw->dev[i]); >> + } >> sky2_suspend(&pdev->dev); >> pci_wake_from_d3(pdev, device_may_wakeup(&pdev->dev)); >> pci_set_power_state(pdev, PCI_D3hot); > > This is not the correct fix, the device is supposed to stay > registered. The correct way to fix this would be to make get_stats > ignore requests for device when suspended. Yes I agree it's not correct fix. But I tried ignoring it in get_stat32 but the crash just moves the bug elsewhere. IMO patching all the places to check the suspended state might be bit heavy ? E.g. with something like below the crash moves to sky2_get_eeprom_len function. sky2_get_eeprom_len+0x10/0x30 dev_ethtool+0x29c/0x1d78 dev_ioctl+0x31c/0x5a8 sock_ioctl+0x2ac/0x310 do_vfs_ioctl+0xa4/0x750 SyS_ioctl+0x8c/0xa0 el0_svc_naked+0x24/0x2 Sorry if I am missing something fundamental, I am bit new to net drivers Regards, Sudeep -->8 } while (gma_read32(hw, port, reg) != val); diff --git i/drivers/net/ethernet/marvell/sky2.c w/drivers/net/ethernet/marvell/sky2.c index ec0a22119e09..d4cfcd89e7e5 100644 --- i/drivers/net/ethernet/marvell/sky2.c +++ w/drivers/net/ethernet/marvell/sky2.c @@ -5175,6 +5175,7 @@ static int sky2_suspend(struct device *dev) } sky2_power_aux(hw); + hw->suspended = true; rtnl_unlock(); return 0; @@ -5198,6 +5199,7 @@ static int sky2_resume(struct device *dev) } rtnl_lock(); + hw->suspended = false; sky2_reset(hw); sky2_all_up(hw); rtnl_unlock(); diff --git i/drivers/net/ethernet/marvell/sky2.h w/drivers/net/ethernet/marvell/sky2.h index ec6dcd80152b..1386e5b635ff 100644 --- i/drivers/net/ethernet/marvell/sky2.h +++ w/drivers/net/ethernet/marvell/sky2.h @@ -2308,6 +2308,7 @@ struct sky2_hw { wait_queue_head_t msi_wait; char irq_name[0]; + bool suspended; }; static inline int sky2_is_copper(const struct sky2_hw *hw) @@ -2378,6 +2379,9 @@ static inline u32 get_stats32(struct sky2_hw *hw, unsigned port, unsigned reg) { u32 val; + if (hw->suspended) + return 0; + do { val = gma_read32(hw, port, reg);