linux-kernel - Re: [PATCH v2] dwc: dra7xx: Print link state to console for debug

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6a82ce2d-3feb-a6eb-d33d-b4b1e3f6b33b@ti.com>
Date:   Tue, 24 Oct 2017 11:48:13 +0530
From:   Kishon Vijay Abraham I <kishon@...com>
To:     Bjorn Helgaas <helgaas@...nel.org>, Faiz Abbas <faiz_abbas@...com>
CC:     <bhelgaas@...gle.com>, <linux-omap@...r.kernel.org>,
        <linux-pci@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] dwc: dra7xx: Print link state to console for debug

Hi Bjorn,

On Monday 23 October 2017 07:34 PM, Bjorn Helgaas wrote:
> 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 <faiz_abbas@...com>
>>>> ---
>>>> 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.,

IMHO both print text and the debug print itself helps to save developer effort.

FWIW:
Acked-by: Kishon Vijay Abraham I <kishon@...com>

Thanks
Kishon