On Thu, May 18, 2017 at 07:43:13PM +0300, Andy Shevchenko wrote: > On Thu, May 18, 2017 at 5:38 PM, Mika Westerberg > <[email protected]> wrote: > > Thunderbolt fabric consists of one or more switches. This fabric is > > called domain and it is controlled by an entity called connection > > manager. The connection manager can be either internal (driven by a > > firmware running on the host controller) or external (software driver). > > This driver currently implements support for the latter. > > > > In order to manage switches and their properties more easily we model > > this domain structure as a Linux bus. Each host controller adds a domain > > device to this bus, and these devices are named as domainN where N > > stands for index or id of the current domain. > > > > We then abstract connection manager specific operations into a new > > structure tb_cm_ops and convert the existing tb.c to fill those > > accordingly. This makes it easier to add support for the internal > > connection manager in subsequent patches. > > One nit below. > > > +static void tb_domain_release(struct device *dev) > > +{ > > + struct tb *tb = container_of(dev, struct tb, dev); > > + > > + if (tb->ctl) > > + tb_ctl_free(tb->ctl); > > The usual pattern is to put such checks inside *_free() type of functions.
tb_ctl_free() and these patterns are from the original code. I just moved the calls from tb.c intto here. No problem doing that change but I suppose that should be done first in a separate patch before this patch. > > + > > + destroy_workqueue(tb->wq); > > + ida_simple_remove(&tb_domain_ida, tb->index); > > + kfree(tb); > > +} > > -- > With Best Regards, > Andy Shevchenko

