From patchwork Thu Oct 25 16:56:01 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Norris X-Patchwork-Id: 194254 X-Patchwork-Delegate: davem@davemloft.net 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 C865F2C0093 for ; Fri, 26 Oct 2012 03:58:01 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935964Ab2JYQ5o (ORCPT ); Thu, 25 Oct 2012 12:57:44 -0400 Received: from mail-pa0-f46.google.com ([209.85.220.46]:54461 "EHLO mail-pa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933975Ab2JYQ53 (ORCPT ); Thu, 25 Oct 2012 12:57:29 -0400 Received: by mail-pa0-f46.google.com with SMTP id hz1so1302104pad.19 for ; Thu, 25 Oct 2012 09:57:29 -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:x-mailer:in-reply-to:references; bh=Tynei49RBd1nFf9cuLOH7WStXqxA/rR7980VpG52VYE=; b=ZyBp+QUIHf9As6qWjT5xoGXxNyxZAskRQJfdxCpMzZb3+WcMQNfQ03xPSJLGJknhTE ixIaUGbR1ccq790X7xxpUoArrRpsmsImD/MFCwc+t0E54WFPIOfMVHrVtpXQRvbVSsTm JAF4ztODoTOZKyvH8HIlQWl1WWps5ejoUaIX5Oh//Az5MMfwhTJPABB6tgMTOGascgzs RtQAmJUrFoYOmQJ9F8rl1WfIWAhFmh4Bhs2Xe4cqhhG3KQuejB8TFz+lBIaq24fIw6ak 1ezQ9+KHWGyc9ew5jeR980QCO5Weie5E2txAfWVTu3ZErIY3nIb5ld92L4gxT8tcBzFw Z0ig== Received: by 10.68.138.166 with SMTP id qr6mr61491161pbb.69.1351184249374; Thu, 25 Oct 2012 09:57:29 -0700 (PDT) Received: from ld-irv-0074.broadcom.com (5520-maca-inet1-outside.broadcom.com. [216.31.211.11]) by mx.google.com with ESMTPS id js9sm2906984pbc.32.2012.10.25.09.57.28 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 25 Oct 2012 09:57:28 -0700 (PDT) From: Brian Norris To: Jeff Garzik Cc: , , Linux Kernel , Brian Norris , Kevin Cernekee , Tejun Heo Subject: [RFC 3/3] libata: don't perform HW activity in devres Date: Thu, 25 Oct 2012 09:56:01 -0700 Message-Id: <1351184161-31433-4-git-send-email-computersforpeace@gmail.com> X-Mailer: git-send-email 1.7.11.3 In-Reply-To: <1351184161-31433-1-git-send-email-computersforpeace@gmail.com> References: <1351184161-31433-1-git-send-email-computersforpeace@gmail.com> Sender: linux-ide-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ide@vger.kernel.org devres functions are intended for simplified cleanup of memory and other software resources on device exit, not for hardware shutdown sequences. In addition, inducing hardware activity at device removal hamstrings some drivers (particularly ahci_platform) so that they cannot totally power off their hardware before removal, as devres cleanup occurs after the driver's exit() sequence. More concretely, I experience the following bus error when using rmmod to remove (and power off) the SATA block on my SoC: # rmmod ahci_platform.ko Data bus error, epc == e07c0ca0, ra == e07c0d24 Oops[#1]: ... Call Trace: [] ahci_stop_engine+0x28/0x84 [libahci] [] ahci_deinit_port+0x28/0xe8 [libahci] [] ahci_port_stop+0x24/0x64 [libahci] [<802dcc28>] ata_host_stop+0x5c/0xc0 [<802b5390>] release_nodes+0x144/0x244 [<802b159c>] __device_release_driver+0x68/0xcc [<802b1fd8>] driver_detach+0xe8/0xf0 [<802b13e0>] bus_remove_driver+0x98/0x128 [<8007b9e4>] sys_delete_module+0x188/0x2d4 [<8000e6fc>] stack_done+0x20/0x40 This hardware activity pattern seems wrong. Therefore, I move ata_host_stop() to simply be called as part of the ata_host_detach() sequence already performed by all SATA drivers at device exit. Signed-off-by: Brian Norris --- drivers/ata/libata-core.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index d31ee55..85dee79 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -5772,9 +5772,8 @@ int ata_slave_link_init(struct ata_port *ap) return 0; } -static void ata_host_stop(struct device *gendev, void *res) +static void ata_host_stop(struct ata_host *host) { - struct ata_host *host = dev_get_drvdata(gendev); int i; WARN_ON(!(host->flags & ATA_HOST_STARTED)); @@ -5858,7 +5857,6 @@ static void ata_finalize_port_ops(struct ata_port_operations *ops) */ int ata_host_start(struct ata_host *host) { - int have_stop = 0; void *start_dr = NULL; int i, rc; @@ -5874,18 +5872,6 @@ int ata_host_start(struct ata_host *host) if (!host->ops && !ata_port_is_dummy(ap)) host->ops = ap->ops; - - if (ap->ops->port_stop) - have_stop = 1; - } - - if (host->ops->host_stop) - have_stop = 1; - - if (have_stop) { - start_dr = devres_alloc(ata_host_stop, 0, GFP_KERNEL); - if (!start_dr) - return -ENOMEM; } for (i = 0; i < host->n_ports; i++) { @@ -6214,6 +6200,8 @@ void ata_host_detach(struct ata_host *host) /* the host is dead now, dissociate ACPI */ ata_acpi_dissociate(host); + + ata_host_stop(host); } #ifdef CONFIG_PCI