Re: [PATCH 10/24] thunderbolt: Read vendor and device name from DROM

[Lukas Wunner]

On Sun, May 21, 2017 at 10:48:19AM +0300, Mika Westerberg wrote:
> On Sun, May 21, 2017 at 07:31:14AM +0200, Lukas Wunner wrote:
> > On Fri, May 19, 2017 at 01:28:36PM +0300, Mika Westerberg wrote:
> > > On Fri, May 19, 2017 at 12:07:10PM +0200, Lukas Wunner wrote:
> > > > If there can be many attributes, should they be stored in a list
> > > > rather than adding a char* pointer for each one to struct tb_switch?
> > > > The latter doesn't scale.
> > > 
> > > I don't think we need other attributes (well, at least right now). The
> > > device/vendor name is useful because that's what we expose to the
> > > userspace for device identification along with the device/vendor ID.
> > 
> > Okay.  It might be worth to log additional attributes with info level.
> 
> I don't think we want to log anything with info level to be honest. The
> driver currently already is pretty noisy so adding even more information
> there just makes it worse ;-)
> 
> I would rather convert debugging information to use tracepoints and get
> rid of the tb_*_info() things completely.

The noisiness has value in that it helps with reverse-engineering:
Just google for dmesg output and check what other machines are
reporting for unknown registers. :-)

If there was public documentation available or Intel would be okay
with answering specific questions (as you've done with the 0x30
attribute id), then the value obviously diminishes.

Can't say anything about converting to tracepoints, that's Andreas'
call.


> The whole DROM content is already available through nvm_active/nvmem
> file under each device (well starting with Alpine Ridge) so the
> userspace can investigate it as much as it likes without spamming the
> kernel dmesg :)

Okay, fair enough, that should indeed suffice.

Thanks,

Lukas