From: Mark Brown <broonie@kernel.org>
To: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
spapothi@codeaurora.org, lgirdwood@gmail.com,
linux-kernel@vger.kernel.org, vkoul@kernel.org,
pierre-louis.bossart@linux.intel.com
Subject: Re: [alsa-devel] [PATCH v7 2/2] ASoC: codecs: add wsa881x amplifier support
Date: Thu, 10 Oct 2019 14:23:14 +0100 [thread overview]
Message-ID: <20191010132314.GQ2036@sirena.org.uk> (raw)
In-Reply-To: <95637c0a-8373-0eda-47e5-ac6e529019e5@linaro.org>
[-- Attachment #1.1: Type: text/plain, Size: 1341 bytes --]
On Thu, Oct 10, 2019 at 10:28:04AM +0100, Srinivas Kandagatla wrote:
> On 09/10/2019 17:35, Mark Brown wrote:
> > On Wed, Oct 09, 2019 at 09:51:08AM +0100, Srinivas Kandagatla wrote:
> > > +static const u8 wsa881x_reg_readable[WSA881X_CACHE_SIZE] = {
> > > +static bool wsa881x_readable_register(struct device *dev, unsigned int reg)
> > > +{
> > > + return wsa881x_reg_readable[reg];
> > There's no bounds check and that array size is not...
> I converted this now to a proper switch statement as other drivers do.
> > > +static struct regmap_config wsa881x_regmap_config = {
> > > + .reg_bits = 32,
> > > + .val_bits = 8,
> > > + .cache_type = REGCACHE_RBTREE,
> > > + .reg_defaults = wsa881x_defaults,
> > > + .num_reg_defaults = ARRAY_SIZE(wsa881x_defaults),
> > > + .max_register = WSA881X_MAX_REGISTER,
> > ...what regmap has as max_register. Uusually you'd render as a
> > switch statement (as you did for volatile) and let the compiler
> > figure out a sensible way to do the lookup.
> Sorry, I did not get your point here.
> Are you saying that we can skip max_register in this regmap config ?
> Then how would max_register in regmap be set?
I'm saying that you appear to be relying on max_register to
verify that you're not overflowing the array bounds but you
max_register is not set to the same thing as the array size.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
next prev parent reply other threads:[~2019-10-10 13:24 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-09 8:51 [alsa-devel] [PATCH v7 0/2] ASoC: codecs: Add WSA881x Smart Speaker amplifier support Srinivas Kandagatla
2019-10-09 8:51 ` [alsa-devel] [PATCH v7 1/2] dt-bindings: ASoC: Add WSA881x bindings Srinivas Kandagatla
2019-10-09 8:51 ` [alsa-devel] [PATCH v7 2/2] ASoC: codecs: add wsa881x amplifier support Srinivas Kandagatla
2019-10-09 16:35 ` Mark Brown
2019-10-09 16:35 ` Mark Brown
2019-10-10 9:28 ` Srinivas Kandagatla
2019-10-10 13:23 ` Mark Brown [this message]
2019-10-10 13:38 ` Srinivas Kandagatla
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191010132314.GQ2036@sirena.org.uk \
--to=broonie@kernel.org \
--cc=alsa-devel@alsa-project.org \
--cc=devicetree@vger.kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=pierre-louis.bossart@linux.intel.com \
--cc=spapothi@codeaurora.org \
--cc=srinivas.kandagatla@linaro.org \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).