linux-next.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mark Brown <broonie@opensource.wolfsonmicro.com>
To: Randy Dunlap <rdunlap@xenotime.net>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>,
	alsa-devel@alsa-project.org, linux-next@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>, Liam Girdwood <lrg@ti.com>
Subject: Re: [alsa-devel] linux-next: Tree for Oct 12 (sound/soc/codecs/wm1250-ev1)
Date: Thu, 13 Oct 2011 22:58:27 +0100	[thread overview]
Message-ID: <20111013215827.GA2866@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <4E972DE1.9020308@xenotime.net>

On Thu, Oct 13, 2011 at 11:28:49AM -0700, Randy Dunlap wrote:

> You are listed as MODULE_AUTHOR().  Can we expect a fix?

I dunno; like I say I don't see an issue in the driver here (the driver
usage of the GPIOs is optional), the GPIO build options are pretty
fiddly and I just wrote the mail below.

> I don't see how stubs help with source lines like:
> struct wm1250_priv {
> 	struct gpio gpios[WM1250_EV1_NUM_GPIOS];
> };

So, clearly there's an issue with the API not stubbing itself out
terribly well here; the obvious solution would appear to be to have a
definition of the struct to go along with the stubs.  I guess you see
some sort of problem with this, though?  It does work well enough for
other similar APIs like regulator_bulk_*() so I'd expect it to work here.

To be honest though I'm more concerned that this is being reported as an
issue in a specific driver than I am about the actual issue (which one
does have to try pretty hard to see, I'd be astonished if someone saw it
outside of deliberate testing).  As a result of the number of randconfig
type tests you do I'd guess you're aware of the APIs like GPIO which are
expected to stub themselves out when disabled.  As with the usage in
this driver there's a huge proportion of users which can optionally use
the API but don't actually need it and which therefore shouldn't have
build time dependencies, and clearly scattering ifdefs throughout the
code isn't great either.  These things do fix the *configs but they
reduce the coverage we get with our automated testing and they make the
code harder to work with.

The main reason I'm pushing back in this way is that as we start to get
more and more embedded hardware in subsystems maintained by people from
a PC background it becomes more and more important that people like you
who do this who do a lot of this kernel wide testing are directing these
issues appropriately by checking to see if what's going wrong is a
result of the driver or if it's a result of one of the service APIs like
GPIO or regulator (and hopefully soon clk) not being transparent enough.

Subsystem maintainers who usually work on PCs likely won't be aware of
what's going on with this sort of stuff except in so far as they get
reports of *config issues since PCs don't generally have to deal with
the haphazard board dependant setup that is so common in the embedded
space and random driver authors aren't going to be reliably familiar
with either that or build coverage type things either as the
configurations that break are generally not too realistic anyway.

I don't mean to get at you personally here - while you do do a lot of
good work here you're not the only person who does this sort of testing
and hence reports issues in this manner, and obviously there's a lot of
maintainers too.  I think it's mostly an education thing but I'm not
sure what the best way to go about spreading the word is, if nothing
else I don't know how to reach all the build coverage people (who I
think are key here).

      reply	other threads:[~2011-10-13 21:58 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-12  6:54 linux-next: Tree for Oct 12 Stephen Rothwell
2011-10-12 15:26 ` Randy Dunlap
2011-10-12 22:26   ` Stephen Rothwell
2011-10-12 22:31     ` Sedat Dilek
2011-10-13  0:46 ` linux-next: Tree for Oct 12 (sound/soc/codecs/wm1250-ev1) Randy Dunlap
2011-10-13 10:24   ` [alsa-devel] " Mark Brown
2011-10-13 18:28     ` Randy Dunlap
2011-10-13 21:58       ` Mark Brown [this message]

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=20111013215827.GA2866@opensource.wolfsonmicro.com \
    --to=broonie@opensource.wolfsonmicro.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-next@vger.kernel.org \
    --cc=lrg@ti.com \
    --cc=rdunlap@xenotime.net \
    --cc=sfr@canb.auug.org.au \
    /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).