Messages in this thread | | | Date | Mon, 10 Sep 2018 15:13:08 +0300 | From | Mika Westerberg <> | Subject | Re: [PATCH 5/5] MAINTAINERS: Add Lukas Wunner as co-maintainer of thunderbolt |
| |
On Mon, Sep 10, 2018 at 12:25:14PM +0200, Lukas Wunner wrote: > 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.
That is actually already on my todo list. What happens here is that on PCs the controller is hot-removed and the driver is removed but it tries to touch hardware after it is already gone. We have a flag for that already so I'm hoping to use that and skip touching hardware if it is set.
> 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.
That I realized also myself after testing the patch bit more. We could print some information that is useful upon device connect/disconnect.
But I don't think dumping the switch structure and ports are useful. I would rather make it to follow USB and print the actual device that was connected (this is the information read from DROM and which is exported in sysfs as well).
So I'm experimenting patch that does something like this when device is connected:
thunderbolt 0-1: device 1:8003 connected thunderbolt 0-1: Apple, Inc Thunderbolt to FireWire Adapter
and when it is disconnected
thunderbolt 0-1: device disconnected
> 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.
Since thunderbolt is now bus and all the devices include embedded struct device we can use dev_*() just like everybody else is doing.
| |