From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752642AbdB1POw (ORCPT ); Tue, 28 Feb 2017 10:14:52 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:55096 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751917AbdB1POp (ORCPT ); Tue, 28 Feb 2017 10:14:45 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 2775860A3B Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=okaya@codeaurora.org Subject: Re: [PATCH V2] PCI/ASPM: reconfigure ASPM following hotplug for POLICY_DEFAULT To: "Patel, Mayurkumar" , "linux-pci@vger.kernel.org" , "timur@codeaurora.org" , "cov@codeaurora.org" References: <1485891928-14573-1-git-send-email-okaya@codeaurora.org> <92EBB4272BF81E4089A7126EC1E7B2846666830C@IRSMSX101.ger.corp.intel.com> Cc: "linux-arm-msm@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , open list From: Sinan Kaya Message-ID: Date: Tue, 28 Feb 2017 10:04:06 -0500 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <92EBB4272BF81E4089A7126EC1E7B2846666830C@IRSMSX101.ger.corp.intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2/28/2017 6:03 AM, Patel, Mayurkumar wrote: >> - /* Save default state */ >> - link->aspm_default = link->aspm_enabled; > But, I am finding a problem with this change, if Policy is set to default, > BIOS enables ASPM L1, but pcie_config_aspm_link() disables ASPM L1 > due to link->aspm_enabled is different than link->aspm_default while > called from pcie_aspm_init_link_state() during hotplug insertion. > Thanks for testing. I think it means the enable count logic I put below is not working. I was trying to figure out when to use saved values vs. the values in registers by looking at the enable_cnt. enable_cnt is 0 during boot on my system. The enable_cnt should have been non-zero following an insertion following remove and default and enabled values should have been identical. Can you print out the values of pdev->aspm_default, link->aspm_default, and link->aspm_enabled in this function along with atomic_read(&pdev->enable_cnt)? > >> + /* >> + * Save default state from FW when enabling ASPM for the first time >> + * during boot by looking at the calculated link->aspm_enabled bits >> + * above and enable_cnt will be zero. >> + * >> + * If this path is getting called for the second/third time >> + * (enable_cnt will be non-zero). Assume that the current state of the >> + * ASPM registers may not necessarily match what FW asked us to do as >> + * in the case of hotplug insertion/removal. >> + */ >> + if (!atomic_read(&pdev->enable_cnt)) >> + pdev->aspm_default = link->aspm_default = link->aspm_enabled; >> + else >> + link->aspm_default = pdev->aspm_default; -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.