From: Mark Brown <broonie@opensource.wolfsonmicro.com>
To: Timur Tabi <timur@freescale.com>
Cc: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>,
alsa-devel@alsa-project.org, patches@opensource.wolfsonmicro.com,
Liam Girdwood <lrg@slimlogic.co.uk>
Subject: Re: [PATCH 2/4] ASoC: soc-cache: Add support for standard register caching
Date: Sun, 24 Oct 2010 14:35:26 -0700 [thread overview]
Message-ID: <20101024213525.GA14256@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <AANLkTi=eWCO71FTj38rS4dehjo+DoLvSGQa6fXgqaaJq@mail.gmail.com>
On Sun, Oct 24, 2010 at 08:18:59AM -0500, Timur Tabi wrote:
> On Sat, Oct 23, 2010 at 1:24 PM, Dimitris Papastamos
> > The only problem I see with the above code, is when
> > codec_drv->reg_word_size > sizeof (unsigned int) but that can't really
> > happen in practice.
Plus if it did happen the rest of the code would fall over fairly badly
since we've got that assumption embedded in the register read and write
APIs.
> I'm going to have to agree with Mark that this code is suspect. I
> understand everything you said, but it makes me nervous. Unless this
> code is in some kind of fast-path, I would prefer to see it rewritten
> to avoid any assumption about the sizes of the types involved.
I think the important thing here is that the code is clear - from a
maintainability so long as it's clear how and why the code works things
should be fine, otherwise we'll have people scratching their heads over
it every time someone looks at the code which is going to be painful.
This could be done with documentation as well as with code changes,
though code changes should definitely be considered.
next prev parent reply other threads:[~2010-10-24 21:35 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-22 14:28 [PATCH 0/4] ASoC: Implement new caching API Dimitris Papastamos
2010-10-22 14:28 ` [PATCH 1/4] ASoC: soc.h: Add new caching API prototypes and hooks Dimitris Papastamos
2010-10-22 15:44 ` Mark Brown
2010-10-22 14:28 ` [PATCH 2/4] ASoC: soc-cache: Add support for standard register caching Dimitris Papastamos
2010-10-22 16:03 ` Mark Brown
2010-10-23 18:24 ` Dimitris Papastamos
2010-10-24 13:18 ` Timur Tabi
2010-10-24 21:35 ` Mark Brown [this message]
2010-10-25 8:09 ` Dimitris Papastamos
2010-10-22 14:28 ` [PATCH 3/4] ASoC: soc-core: Adapt soc-core to fit the new caching API Dimitris Papastamos
2010-10-22 14:28 ` [PATCH 4/4] ASoC: soc-cache: Add support for LZO based register caching Dimitris Papastamos
2010-10-22 16:18 ` Mark Brown
2010-10-23 18:28 ` Dimitris Papastamos
2010-10-25 18:13 ` Mark Brown
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=20101024213525.GA14256@opensource.wolfsonmicro.com \
--to=broonie@opensource.wolfsonmicro.com \
--cc=alsa-devel@alsa-project.org \
--cc=dp@opensource.wolfsonmicro.com \
--cc=lrg@slimlogic.co.uk \
--cc=patches@opensource.wolfsonmicro.com \
--cc=timur@freescale.com \
/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).