From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932336AbdJWOER (ORCPT ); Mon, 23 Oct 2017 10:04:17 -0400 Received: from mail.kernel.org ([198.145.29.99]:59896 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932191AbdJWOEP (ORCPT ); Mon, 23 Oct 2017 10:04:15 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B054A21903 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=helgaas@kernel.org Date: Mon, 23 Oct 2017 09:04:11 -0500 From: Bjorn Helgaas To: Faiz Abbas Cc: kishon@ti.com, bhelgaas@google.com, linux-omap@vger.kernel.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] dwc: dra7xx: Print link state to console for debug Message-ID: <20171023140411.GB9092@bhelgaas-glaptop.roam.corp.google.com> References: <1508417009-30869-1-git-send-email-faiz_abbas@ti.com> <20171020230952.GZ6332@bhelgaas-glaptop.roam.corp.google.com> <3692fbeb-bf8a-6de6-898b-551d899e9e55@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3692fbeb-bf8a-6de6-898b-551d899e9e55@ti.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Oct 23, 2017 at 03:59:49PM +0530, Faiz Abbas wrote: > On Saturday 21 October 2017 04:39 AM, Bjorn Helgaas wrote: > > On Thu, Oct 19, 2017 at 06:13:29PM +0530, Faiz Abbas wrote: > >> Enable support for printing the LTSSM link state for debugging PCI > >> when link is down. > >> > >> Signed-off-by: Faiz Abbas > >> --- > >> v2: > >> 1. Changed dev_err() to dev_dbg() > >> 2. Changed static char array to static const char * const > >> 3. format changes > > > > I'm not really sure how much debug help we want to carry around in the > > mainline kernel. End users aren't going to use this; it seems like > > more of a lab tool, and in situations like that you usually end up > > carrying around some out-of-tree patches for a while anyway. But I > > can probably be convinced either way. > > It'll be easier to support customers if they can tell us what the state > of the link is by just changing the log level. We won't have to send a > debug patch to the customer to find that out. That still sounds like a lab debug situation to me. Regular customers do not debug things at the level of the link state. I'm not aware of any other drivers that do this, so including this hints that this driver/hardware is not very mature. Printing text certainly *looks* nice, but it adds a lot of code and I'm not sure how much actual value they add. Just printing a hex value might be more reliable in terms of communicating it accurately back to you. E.g., it might be easier to lose the distinction between DISABLED_ENTRY and DISABLED_IDLE than between 0x745f and 0x7463, especially in a phone situation. Anyway, if Kishon acks this, I'll apply it. One nit: please do the "link up" test once, e.g., link_up = !!(reg & LINK_UP); if (!link_up) { cmd_reg = dra7xx_pcie_readl(...); dev_dbg(...); } return link_up;