[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1493086864.25766.266.camel@kernel.crashing.org>
Date: Tue, 25 Apr 2017 12:21:04 +1000
From: Benjamin Herrenschmidt <benh@...nel.crashing.org>
To: Brendan Higgins <brendanhiggins@...gle.com>, wsa@...-dreams.de,
robh+dt@...nel.org, mark.rutland@....com, tglx@...utronix.de,
jason@...edaemon.net, marc.zyngier@....com, joel@....id.au,
vz@...ia.com, mouse@...c.ru, clg@...d.org
Cc: linux-i2c@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, openbmc@...ts.ozlabs.org
Subject: Re: [PATCH v7 4/5] i2c: aspeed: added driver for Aspeed I2C
On Mon, 2017-04-24 at 11:18 -0700, Brendan Higgins wrote:
> +static int __aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
> + struct platform_device *pdev)
> +{
Minor nit ... I'm really not fan of those underscores.
We use __ functions in some cases in the kernel for low level
helpers, usually when it's a low level variant of an existing
function or an "unlocked" variant, but I don't think generalizing
it to pretty much everything in the driver is worthwhile here.
If you want to be explicit about locking, I would suggest you
use a comment in front of the function explaining if it
expects to be called with the lock held.
We tend to only do that when *both* functions exist and one is
implemented in term of the other.
Cheers,
Ben.
Powered by blists - more mailing lists