Date:   Fri, 24 Dec 2021 13:28:57 +0000
From:   Mark Brown <broonie@kernel.org>
To:     Quan Nguyen <quan@os.amperecomputing.com>
Cc:     Joel Stanley <joel@jms.id.au>, Andrew Jeffery <andrew@aj.id.au>,
        Jean Delvare <jdelvare@suse.com>,
        Guenter Roeck <linux@roeck-us.net>,
        Rob Herring <robh+dt@kernel.org>,
        Lee Jones <lee.jones@linaro.org>,
        Jonathan Corbet <corbet@lwn.net>, linux-hwmon@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-doc@vger.kernel.org, linux-aspeed@lists.ozlabs.org,
        openbmc@lists.ozlabs.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J . Wysocki" <rafael@kernel.org>,
        Derek Kiernan <derek.kiernan@xilinx.com>,
        Dragan Cvetic <dragan.cvetic@xilinx.com>,
        Arnd Bergmann <arnd@arndb.de>,
        Gustavo Pimentel <Gustavo.Pimentel@synopsys.com>,
        Open Source Submission <patches@amperecomputing.com>,
        Phong Vo <phong@os.amperecomputing.com>,
        "Thang Q . Nguyen" <thang@os.amperecomputing.com>
Subject: Re: [PATCH v6 6/9] misc: smpro-errmon: Add Ampere's SMpro error
 monitor driver
Message-ID: <YcXLGYWx4gYq/Y+J@sirena.org.uk>
References: <20211224041352.29405-1-quan@os.amperecomputing.com>
 <20211224041352.29405-7-quan@os.amperecomputing.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="p1TkcOZIYZMx6FNy"
Content-Disposition: inline
In-Reply-To: <20211224041352.29405-7-quan@os.amperecomputing.com>
Precedence: bulk


--p1TkcOZIYZMx6FNy
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Fri, Dec 24, 2021 at 11:13:49AM +0700, Quan Nguyen wrote:

> +static int read_i2c_block_data(struct i2c_client *client, u16 address, u16 length, u8 *data)
> +{
> +	unsigned char outbuf[MAX_READ_BLOCK_LENGTH];
> +	unsigned char inbuf[2];
> +	struct i2c_msg msgs[2];
> +	ssize_t ret;
> +
> +	inbuf[0] = (address & 0xff);
> +	inbuf[1] = length;
> +
> +	msgs[0].addr = client->addr;
> +	msgs[0].flags = client->flags & I2C_M_TEN;
> +	msgs[0].len = 2;
> +	msgs[0].buf = inbuf;
> +
> +	msgs[1].addr = client->addr;
> +	msgs[1].flags = (client->flags  & I2C_M_TEN) | I2C_M_RD;
> +	msgs[1].len = length;
> +	msgs[1].buf = outbuf;
> +
> +	ret = i2c_transfer(client->adapter, msgs, NUM_I2C_MESSAGES);
> +	if (ret < 0)

regmap already supports bulk data access so it's not clear why we
would open code this.  Given that this is a single I2C transfer
it's also unclear why we'd need to export the regmap lock - a
single I2C transfer is atomic at the bus level so the regmap lock
isn't providing any additional concurrency protection.

--p1TkcOZIYZMx6FNy
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmHFyxkACgkQJNaLcl1U
h9DP9gf/Q0bcZ3lkRyxjxhgET9fowIKusHVDbL3su16XSlv5Rt4Tih9Rf9nu8FBm
oHmRlAtBCP9ZuA+7I743tz+/H2BeISzoGI8aF3h+MPdh2darno04pVGedC2XR1jn
9Q9cu4/TZhXOt7pazpkfUCmxHT4kn3tMDZ8u9XSWHpDX9mh80/IHwyZ6NFgMruKF
RHWAu9RjAeUCOy8tlembNTm+G9EMDKFS2XI44T3/BtW8u3c7WyiMqdr3EICCyHbd
PjXo5uAjGCljZoVoJ7L0+zUYjzvdQvYuRZ+kQfXEX8MPnE7WyV1MREkMSwl/dqXN
0M6wsXHhdztUM4b2EtWrp6M0kem1tg==
=hzut
-----END PGP SIGNATURE-----

--p1TkcOZIYZMx6FNy--