From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755451AbdESIUU (ORCPT ); Fri, 19 May 2017 04:20:20 -0400 Received: from mga11.intel.com ([192.55.52.93]:53215 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752689AbdESIUP (ORCPT ); Fri, 19 May 2017 04:20:15 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.38,363,1491289200"; d="scan'208";a="970634455" Date: Fri, 19 May 2017 11:15:26 +0300 From: Mika Westerberg To: Andy Shevchenko Cc: Greg Kroah-Hartman , Andreas Noever , Michael Jamet , Yehezkel Bernat , Lukas Wunner , Amir Levy , Andy Lutomirski , Mario Limonciello , Jared.Dominguez@dell.com, Andy Shevchenko , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH 06/24] thunderbolt: Introduce thunderbolt bus and connection manager Message-ID: <20170519081526.GD3267@lahna.fi.intel.com> References: <20170518143914.60902-1-mika.westerberg@linux.intel.com> <20170518143914.60902-7-mika.westerberg@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, May 18, 2017 at 07:43:13PM +0300, Andy Shevchenko wrote: > On Thu, May 18, 2017 at 5:38 PM, Mika Westerberg > 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