From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755869AbcARQGN (ORCPT ); Mon, 18 Jan 2016 11:06:13 -0500 Received: from mezzanine.sirena.org.uk ([106.187.55.193]:60234 "EHLO mezzanine.sirena.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755690AbcARQGJ (ORCPT ); Mon, 18 Jan 2016 11:06:09 -0500 Date: Mon, 18 Jan 2016 16:05:57 +0000 From: Mark Brown To: Stefan Agner Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org Message-ID: <20160118160557.GG6588@sirena.org.uk> References: <1453099976-9016-1-git-send-email-stefan@agner.ch> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="OqG5N5f8gK0cTFut" Content-Disposition: inline In-Reply-To: <1453099976-9016-1-git-send-email-stefan@agner.ch> X-Cookie: APL hackers do it in the quad. User-Agent: Mutt/1.5.24 (2015-08-30) X-SA-Exim-Connect-IP: 2a01:348:6:8808:fab::3 X-SA-Exim-Mail-From: broonie@sirena.org.uk Subject: Re: [RFC] regmap: clairify max_register meaning X-SA-Exim-Version: 4.2.1 (built Mon, 26 Dec 2011 16:24:06 +0000) X-SA-Exim-Scanned: Yes (on mezzanine.sirena.org.uk) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --OqG5N5f8gK0cTFut Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Sun, Jan 17, 2016 at 10:52:56PM -0800, Stefan Agner wrote: Please submit one change per patch. You've mixed a change to the documentation with a code here and it's really unclear what either of them are supposed to do. > Maybe I see something completely wrong here, but to me it seems > that the regcache-flat.c is the only code interpreting max_register > as register indexes rather then the maximum relative address... I don't think I understand what the above means, sorry. What is a "relative address" in the context of a register map? > At least regmap.c compares with range_max, which seems to use > addresses. That's a fairly large source file, can you be more specific please? > - map->cache = kcalloc(map->max_register + 1, sizeof(unsigned int), > - GFP_KERNEL); > + map->cache = kzalloc(map->max_register + map->reg_stride, GFP_KERNEL); This isn't very obvious and appears broken for a lot of devices. It does two things, it converts from allocating an array of unsigned integers to an array of bytes and it replaces the handling of address 0 by adding an extra element to the array with adding stride bytes on the end of the array. This means that for a regmap with 16 bit values and a stride of 1 we'll end up allocating nowhere the space we need as we'll only allocate one byte per register plus an extra byte at the end but the implementation of the flat cache is treating the cache as an array of unsigned ints so needs at least four bytes per register. It should wash out as the same thing if the stride is equal to the size of an unsigned int but most other cases will be broken. Your changelog doesn't actually say this but I think what this is trying to do is make the cache more memory efficient by not allocating space for the registers that don't exist due to striding (ie, if we have a 4 byte stride then currently we'll allocate space for 4 times as many registers as actually exist). That's a reasonable goal but I don't immediately have a good idea for doing it bearing in mind that we are very performance sensitive here. > - * @max_register: Optional, specifies the maximum valid register index. > + * @max_register: Optional, specifies the maximum valid register address. This is reasonable (index and address are synonyms here). --OqG5N5f8gK0cTFut Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJWnQ1lAAoJECTWi3JdVIfQYnAH/15C9bZVt0KORli2Kx2nUUBx sy0bZpqCJRWdGL8BlEig6jv2qvZUha1m2AOS7vHU0iTzxJ9JIYuqKpa3i79HKSXV AYrE4jOhwBZa9elpGAunDAh+RYnD167oxbvlwHPjOYWUP0hTLho3iiBI1y6Ccqxy yLxJziPqVaJBdUFnmUO5cdWyZLSR9a6mxiy/9CNvNNNp/DD/udskyuYSw+beYle5 KnIGUG1NsH4fevzvT9fI0CPTmtXXUhJhs2NSpGS0Cm6ALppM2bgIAuIkVgUUZ2L0 ZZuxPbMyeHMcQUqlpI5Fj2ow41qgn5gC+xbN+Gz2siAo9066xXFB0z5CODK71DU= =SyqF -----END PGP SIGNATURE----- --OqG5N5f8gK0cTFut--