* [PATCH] PCI: dwc: dra7xx: Print link state to console for debug
@ 2017-10-17 5:43 Faiz Abbas
2017-10-17 6:35 ` Joe Perches
0 siblings, 1 reply; 5+ messages in thread
From: Faiz Abbas @ 2017-10-17 5:43 UTC (permalink / raw)
To: kishon; +Cc: bhelgaas, linux-omap, linux-pci, linux-kernel, faiz_abbas
Enable support for printing the LTSSM link state for debugging PCI
when link is down.
Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
drivers/pci/dwc/pci-dra7xx.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
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"
+};
+
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]);
+ }
return !!(reg & LINK_UP);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] PCI: dwc: dra7xx: Print link state to console for debug
2017-10-17 5:43 [PATCH] PCI: dwc: dra7xx: Print link state to console for debug Faiz Abbas
@ 2017-10-17 6:35 ` Joe Perches
2017-10-17 14:45 ` David Laight
2017-10-18 6:16 ` Faiz Abbas
0 siblings, 2 replies; 5+ messages in thread
From: Joe Perches @ 2017-10-17 6:35 UTC (permalink / raw)
To: Faiz Abbas, kishon; +Cc: bhelgaas, linux-omap, linux-pci, linux-kernel
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",
...
};
> +};
> +
> 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);
> }
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] PCI: dwc: dra7xx: Print link state to console for debug
2017-10-17 6:35 ` Joe Perches
@ 2017-10-17 14:45 ` David Laight
2017-10-19 13:23 ` Faiz Abbas
2017-10-18 6:16 ` Faiz Abbas
1 sibling, 1 reply; 5+ messages in thread
From: David Laight @ 2017-10-17 14:45 UTC (permalink / raw)
To: 'Joe Perches', Faiz Abbas, kishon@ti.com
Cc: bhelgaas@google.com, linux-omap@vger.kernel.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
From: Joe Perches
> Sent: 17 October 2017 07:35
> 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",
> ...
Or 4 aligned columns.
Better still used named initialisers:
[xxx_DETECT_QUIET] = "DETECT_QUIET",
you really don't want an 'off-by-one' in that list.
David
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] PCI: dwc: dra7xx: Print link state to console for debug
2017-10-17 6:35 ` Joe Perches
2017-10-17 14:45 ` David Laight
@ 2017-10-18 6:16 ` Faiz Abbas
1 sibling, 0 replies; 5+ messages in thread
From: Faiz Abbas @ 2017-10-18 6:16 UTC (permalink / raw)
To: Joe Perches, kishon; +Cc: bhelgaas, linux-omap, linux-pci, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] PCI: dwc: dra7xx: Print link state to console for debug
2017-10-17 14:45 ` David Laight
@ 2017-10-19 13:23 ` Faiz Abbas
0 siblings, 0 replies; 5+ messages in thread
From: Faiz Abbas @ 2017-10-19 13:23 UTC (permalink / raw)
To: David Laight, 'Joe Perches', kishon@ti.com
Cc: bhelgaas@google.com, linux-omap@vger.kernel.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Hi,
On Tuesday 17 October 2017 08:15 PM, David Laight wrote:
> From: Joe Perches
>> Sent: 17 October 2017 07:35
>> 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",
>> ...
>
> Or 4 aligned columns.
4 aligned columns are difficult as the strings are of different sizes.
> Better still used named initialisers:
> [xxx_DETECT_QUIET] = "DETECT_QUIET",
> you really don't want an 'off-by-one' in that list.
Sounds good. Let me see how I can implement this.
Thanks,
Faiz
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-10-19 13:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-17 5:43 [PATCH] PCI: dwc: dra7xx: Print link state to console for debug Faiz Abbas
2017-10-17 6:35 ` Joe Perches
2017-10-17 14:45 ` David Laight
2017-10-19 13:23 ` Faiz Abbas
2017-10-18 6:16 ` Faiz Abbas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).