Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 10 Sep 2018 12:25:14 +0200
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
Message-ID: <20180910102514.tjwortosyb32n5me@wunner.de>
References: <cover.1536517047.git.lukas@wunner.de>
 <76fccab34a66023c08b71a864a9fea77daac9742.1536517047.git.lukas@wunner.de>
 <20180910093333.GI14465@lahna.fi.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20180910093333.GI14465@lahna.fi.intel.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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