From patchwork Wed Jul 11 16:21:20 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 170489 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 442E22C01CC for ; Thu, 12 Jul 2012 02:21:23 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755925Ab2GKQVW (ORCPT ); Wed, 11 Jul 2012 12:21:22 -0400 Received: from mail-gg0-f202.google.com ([209.85.161.202]:44949 "EHLO mail-gg0-f202.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755957Ab2GKQVV (ORCPT ); Wed, 11 Jul 2012 12:21:21 -0400 Received: by ggeh3 with SMTP id h3so134849gge.1 for ; Wed, 11 Jul 2012 09:21:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=m2Z6ZuEenymFBt1uGs4mmjZelNabCacZ+bywy3X2Haw=; b=osLYQu48LSz3MjCOynnGBd50rQpPV27h74mb1mTw18wgihr9KhtuvqWhw7lols26Ls GvLkg7KCtDQKBwWbkpAMLfx1d4OT2ZQSEWd296Edzz0OKInb1wRkVwIhVaumxpardIT/ eR3plkUxj5+Z9X5QDezlBwMbs0w+e6J+yfW88hj1CwmPm4kSa3kONDAsecxsQ9gpHfzb 3Mh/WTaYA0EHN8vONeC3bmBZxz1MTM7Xa5RBLXnxN5ZijOso8KjwndnfuCCeI2hfJIyG OMp61O91ngJpCg3mkZU2CxJlPyQ9ePG3Qx3cq8glZiwsFkDk9huRLw+l9DcYiyAga1kw G65Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent :x-gm-message-state; bh=m2Z6ZuEenymFBt1uGs4mmjZelNabCacZ+bywy3X2Haw=; b=jHW+sxxBM0dn5bp8m8hDirszogAALunsZ2BgM+5JsUcOQ0TPDKVQM+FjJ/+EvTY0cE B39zYByEpzEZGMUWCtC8aHRy++Eo6n1W8n3BNac3SEdCj9zzsBr5p4aD+sPshCaygfRv aiexQ7Vge5Owb1Npv2gDUpwWYe7sSnaulbG5iiXJbVqNcIdKdEmWKWYBN0DReOe+7p1e AB8Dtuf1Bz6JK3iCtM1HDrtONdsqdSjCB/fbp+UwfLDTX7CEYElB5e4TK2dilVFK3952 ISRP0UjZ9rH+4y/hM/A7z1mc5c3xKwiEF5Lb0B032HuvhBdmoYeMJwlPRUbCjE1ws30J 8V+Q== Received: by 10.236.83.111 with SMTP id p75mr81550187yhe.5.1342023680690; Wed, 11 Jul 2012 09:21:20 -0700 (PDT) Received: by 10.236.83.111 with SMTP id p75mr81550177yhe.5.1342023680613; Wed, 11 Jul 2012 09:21:20 -0700 (PDT) Received: from wpzn4.hot.corp.google.com (216-239-44-65.google.com [216.239.44.65]) by gmr-mx.google.com with ESMTPS id u67si636814yhi.7.2012.07.11.09.21.20 (version=TLSv1/SSLv3 cipher=AES128-SHA); Wed, 11 Jul 2012 09:21:20 -0700 (PDT) Received: from bhelgaas.mtv.corp.google.com (bhelgaas.mtv.corp.google.com [172.18.96.155]) by wpzn4.hot.corp.google.com (Postfix) with ESMTP id 80D911E0043; Wed, 11 Jul 2012 09:21:20 -0700 (PDT) Received: by bhelgaas.mtv.corp.google.com (Postfix, from userid 131485) id 1BAB4180828; Wed, 11 Jul 2012 09:21:20 -0700 (PDT) Date: Wed, 11 Jul 2012 10:21:20 -0600 From: Bjorn Helgaas To: Yinghai Lu Cc: linux-pci@vger.kernel.org, Kenji Kaneshige Subject: Re: [PATCH 2/5] pciehp: Don't enable presence notification while surprise removal is not supported. Message-ID: <20120711162120.GA17988@google.com> References: <1340437325-29282-1-git-send-email-yinghai@kernel.org> <1340437325-29282-3-git-send-email-yinghai@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) X-Gm-Message-State: ALoCoQmJY11Cnnq4W9FkfLGAi6Mlt+tY6bu74N5NmhgIc1UJGypFDj8XmUlpsZxGTv2zWnAFAMiqByqfv2ebqSAhwYQNBADLAOFZ3ofGY0UZocBiWTnybEaJYKYHBNJzy/ZN3KRGJxLpFi0qP8DmXUb8aimClboBRDD76dZP6o1bp0ux0Uh63Ny8L27Dsw2YKp6+JrtH0DN1 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On Tue, Jul 10, 2012 at 04:56:15PM -0600, Bjorn Helgaas wrote: > On Sat, Jun 23, 2012 at 1:42 AM, Yinghai Lu wrote: > > If surprise removal is not supported, that event get dropped later. > > So there is no point to enable that. > > > > Also some sick chipset have those bit flip around when the card is not present. > > and make log full of useless warning. > > HP_SUPR_RM tests the Slot Capabilities "Hot-Plug Surprise" bit, which > indicates that an adapter might be *removed* without prior > notification (sec 7.8.9). > > PCI_EXP_SLTCTL_PDCE is the Slot Control "Presence Detect Changed > Enable" bit, which enables interrupts for any change in the Slot > Status "Presence Detect State", i.e., we may get interrupts for either > add or remove events. > > In interrupt_event_handler(), we drop both add and remove events if > !HP_SUPR_RM(). Specifically, we drop *add* events if surprise > *removal* isn't supported. That seems strange -- just from reading > the spec, it seems that a surprise *add* could occur even if the > "Hot-Plug Surprise" bit is not set. > > So I'm not convinced that we should even bother looking at the > "Hot-Plug Surprise" bit... Maybe we'd be better off if we just > removed that HP_SUPR_RM() test in interrupt_event_handler() and always > called handle_surprise_event(). What bad things would happen if we did the following? I have no way to test this, but I don't understand what benefit there is in testing HP_SUPR_RM() here. --- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c index 27f4429..4bbe257 100644 --- a/drivers/pci/hotplug/pciehp_ctrl.c +++ b/drivers/pci/hotplug/pciehp_ctrl.c @@ -463,9 +463,8 @@ static void interrupt_event_handler(struct work_struct *work) break; case INT_PRESENCE_ON: case INT_PRESENCE_OFF: - if (!HP_SUPR_RM(ctrl)) - break; - ctrl_dbg(ctrl, "Surprise Removal\n"); + ctrl_dbg(ctrl, "Presence Detect changed (now %spresent)\n", + info->event_type == INT_PRESENCE_OFF ? "not " : ""); handle_surprise_event(p_slot); break; default: