From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757977AbdJRGQo (ORCPT ); Wed, 18 Oct 2017 02:16:44 -0400 Received: from lelnx193.ext.ti.com ([198.47.27.77]:61912 "EHLO lelnx193.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757031AbdJRGQn (ORCPT ); Wed, 18 Oct 2017 02:16:43 -0400 Subject: Re: [PATCH] PCI: dwc: dra7xx: Print link state to console for debug To: Joe Perches , CC: , , , References: <1508219031-25592-1-git-send-email-faiz_abbas@ti.com> <1508222128.6530.22.camel@perches.com> From: Faiz Abbas Message-ID: <7ffa762b-292a-be10-b959-4ebb2312bed6@ti.com> Date: Wed, 18 Oct 2017 11:46:55 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <1508222128.6530.22.camel@perches.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday 17 October 2017 12:05 PM, Joe Perches wrote: > On Tue, 2017-10-17 at 11:13 +0530, Faiz Abbas wrote: >> Enable support for printing the LTSSM link state for debugging PCI >> when link is down. > [] >> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c >> index 34427a6..7b150b0 100644 >> --- a/drivers/pci/dwc/pci-dra7xx.c >> +++ b/drivers/pci/dwc/pci-dra7xx.c >> @@ -98,6 +98,18 @@ struct dra7xx_pcie_of_data { >> >> #define to_dra7xx_pcie(x) dev_get_drvdata((x)->dev) >> >> +static char state[][20] = { >> + "DETECT_QUIET", "DETECT_ACT", "POLL_ACTIVE", "POLL_COMPLIANCE", >> + "POLL_CONFIG", "PRE_DETECT_QUIET", "DETECT_WAIT", "CFG_LINKWD_START", >> + "CFG_LINKWD_ACEPT", "CFG_LANENUM_WAIT", "CFG_LANENUM_ACEPT", >> + "CFG_COMPLETE", "CFG_IDLE", "RCVRY_LOCK", "RCVRY_SPEED", >> + "RCVRY_RCVRCFG", "RCVRY_IDLE", "L0", "L0S", "L123_SEND_EIDLE", >> + "L1_IDLE", "L2_IDLE", "L2_WAKE", "DISABLED_ENTRY", "DISABLED_IDLE", >> + "DISABLED", "LPBK_ENTRY", "LPBK_ACTIVE", "LPBK_EXIT", >> + "LPBK_EXIT_TIMEOUT", "HOT_RESET_ENTRY", "HOT_RESET", "RCVRY_EQ0", >> + "RCVRY_EQ1", "RCVRY_EQ2", "RCVRY_EQ3" > > what's wrong with using the far more typical > > static const char *link_state[] = { > "DETECT_QUIET", > ... > }; Too many lines? > >> +}; >> + >> static inline u32 dra7xx_pcie_readl(struct dra7xx_pcie *pcie, u32 offset) >> { >> return readl(pcie->base + offset); >> @@ -118,6 +130,15 @@ static int dra7xx_pcie_link_up(struct dw_pcie *pci) >> { >> struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pci); >> u32 reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_PHY_CS); >> + u32 cmd_reg; >> + u32 ltssm_state; >> + >> + if (!(reg & LINK_UP)) { >> + cmd_reg = dra7xx_pcie_readl(dra7xx, >> + PCIECTRL_DRA7XX_CONF_DEVICE_CMD); >> + ltssm_state = (cmd_reg & GENMASK(7, 2)) >> 2; >> + dev_err(pci->dev, "Link state:%s\n", state[ltssm_state]); > > and why is this a dev_err and not dev_info? > and if it's really for debugging, why not dev_dbg? > >> + } >> >> return !!(reg & LINK_UP); >> } Will change to dev_dbg. Thanks, Faiz