All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andreas Noever <andreas.noever@gmail.com>,
	Michael Jamet <michael.jamet@intel.com>,
	Yehezkel Bernat <YehezkelShB@gmail.com>,
	Stephen Hemminger <stephen@networkplumber.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 5/5] MAINTAINERS: Add Lukas Wunner as co-maintainer of thunderbolt
Date: Mon, 10 Sep 2018 12:25:14 +0200	[thread overview]
Message-ID: <20180910102514.tjwortosyb32n5me@wunner.de> (raw)
In-Reply-To: <20180910093333.GI14465@lahna.fi.intel.com>

On Mon, Sep 10, 2018 at 12:33:33PM +0300, Mika Westerberg wrote:
> The reason for making the driver less verbose comes from direct feedback
> from the community. For example:
> 
>   https://lkml.org/lkml/2017/10/31/864

I am not opposed to muting messages to KERN_DEBUG severity which merely
report what the driver is doing such as:

    "control channel created"
    "control channel starting..."

However messages should NOT be muted which report register contents
or register changes unless those registers are *fully* documented and
register changes are known to work *reliably*.  The URL you're referring
to above provides an example where that's not the case:

    "disabling interrupt at register 0x38200 bit 12 (0xffffffff -> 0xffffefff)"

Something is broken here, the register was read as "all ones".
This doesn't seem to work as reliable as it should and in that case
please don't mute the message until we know it's fixed and always
works.

Also, it is quite customary and serves a useful purpose to report
devices at KERN_INFO severity as they're enumerated.  E.g. the PCI
bus logs messages for each enumerated device, pciehp logs the
port's capabilities on probe, and so on.  Therefore please do not
mute the enumeration of switches and their ports.  If you find the
messages too noisy, feel free to condense the data reported for each
port to 1 or 2 lines.

We currently print "Port ..." on enumeration, but use the syntax
"<route string>:<port number>" for other port-related messages
printed with tb_port_*().  It might be beneficial to use a single
syntax consistently.

In one of the patches I've posted yesterday, I'm logging unknown
data that is stashed in the DROM for PCI ports.  To make sense of
that data, at least one line for each port needs to be logged.
(Such that we know which data pertains to which port.)

I don't really like spamming the log with this unknown data that
needs to be reverse-engineered.  I hate it.  You would do everyone
a favor if you could divulge the meaning of such unknown registers.
Then we don't need to log it in the first place.

Thanks,

Lukas

  reply	other threads:[~2018-09-10 10:25 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-09 21:42 [PATCH 0/5] Thunderbolt material for v4.20 Lukas Wunner
2018-09-09 21:42 ` [PATCH 4/5] thunderbolt: Correlate PCI devices with Thunderbolt ports Lukas Wunner
2018-09-10  9:44   ` Mika Westerberg
2018-09-13  9:43     ` Yehezkel Bernat
2018-09-13  9:57       ` Mika Westerberg
2018-09-09 21:42 ` [PATCH 3/5] thunderbolt: Move upstream_port to struct tb Lukas Wunner
2018-09-09 21:42 ` [PATCH 2/5] thunderbolt: Obtain PCI slot number from DROM Lukas Wunner
2018-09-10  9:52   ` Mika Westerberg
2018-09-09 21:42 ` [PATCH 5/5] MAINTAINERS: Add Lukas Wunner as co-maintainer of thunderbolt Lukas Wunner
2018-09-10  9:33   ` Mika Westerberg
2018-09-10 10:25     ` Lukas Wunner [this message]
2018-09-10 12:13       ` Mika Westerberg
2018-09-13  8:58     ` Mika Westerberg
2018-09-13  9:43       ` Greg Kroah-Hartman
2018-09-17 22:34       ` Andreas Noever
2018-09-19 10:16         ` Mika Westerberg
2018-09-09 21:42 ` [PATCH 1/5] thunderbolt: Skip disabled ports on tunnel establishment Lukas Wunner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180910102514.tjwortosyb32n5me@wunner.de \
    --to=lukas@wunner.de \
    --cc=YehezkelShB@gmail.com \
    --cc=andreas.noever@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michael.jamet@intel.com \
    --cc=mika.westerberg@linux.intel.com \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.