Re: [PATCH V3 3/6] slimbus: Add messaging APIs to slimbus framework

From: Mark Brown <broonie@kernel.org>
To: Sagar Dharia <sdharia@codeaurora.org>
Cc: gregkh@linuxfoundation.org, bp@suse.de, poeschel@lemonage.de,
	treding@nvidia.com, gong.chen@linux.intel.com,
	andreas.noever@gmail.com, alan@linux.intel.com,
	mathieu.poirier@linaro.org, daniel@ffwll.ch, oded.gabbay@amd.com,
	jkosina@suse.cz, sharon.dvir1@mail.huji.ac.il, joe@perches.com,
	davem@davemloft.net, james.hogan@imgtec.com,
	michael.opdenacker@free-electrons.com,
	daniel.thompson@linaro.org, robh+dt@kernel.org,
	pawel.moll@arm.com, mark.rutland@arm.com,
	ijc+devicetree@hellion.org.uk, galak@codeaurora.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	kheitke@audience.com, mlocke@codeaurora.org,
	agross@codeaurora.org, linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH V3 3/6] slimbus: Add messaging APIs to slimbus framework
Date: Fri, 14 Aug 2015 19:33:53 +0100	[thread overview]
Message-ID: <20150814183353.GA30451@sirena.org.uk> (raw)
In-Reply-To: <20150814182525.GD10748@sirena.org.uk>

[-- Attachment #1: Type: text/plain, Size: 1004 bytes --]

On Fri, Aug 14, 2015 at 07:25:25PM +0100, Mark Brown wrote:
> On Mon, Aug 03, 2015 at 12:59:47AM -0600, Sagar Dharia wrote:

> > +	sema_init(&ctrl->tx_sem, (ctrl->tx.n - 1));

> This sempahore counts the number of free slots in the statically
> allocated list of transfers we have allocated.  It would be good to see
> some discussion in the changelog as to why we've got this statically
> allocated pool rather than just dynamically allocating them as needed -
> I can see a performance argument there but it'd be good to have the
> thinking documented.

One other related thing here: we're keeping the buffers permanently
coherently mapped rather than only mapping them to the device as needed.
Is this the best approach?  On the one hand it increases the cost to the
CPU when setting things up and reading data back, on the other hand it
avoids mapping operations.

Since it's becoming part of the driver API with this it bears some
thought, it will be a little annoying (and fiddly) to change later.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]