All of lore.kernel.org
 help / color / mirror / Atom feed
From: Troy Kisky <troy.kisky@boundarydevices.com>
To: Mark Brown <broonie@sirena.org.uk>
Cc: alsa-devel@alsa-project.org, Arun KS <arunks@mistralsolutions.com>
Subject: Re: [PATCH 0/5] [RFC] ALSA ASoC Support for TLVaic23b audio codec
Date: Mon, 29 Sep 2008 12:07:53 -0700	[thread overview]
Message-ID: <48E12789.50006@boundarydevices.com> (raw)
In-Reply-To: <20080929183045.GA21010@sirena.org.uk>

Mark Brown wrote:
> On Mon, Sep 29, 2008 at 10:58:43AM -0700, Troy Kisky wrote:
> In the device model the board registers the *device*, the codec (or
> whatever) driver registers the *driver* - the two are separated.  The
> driver core then instantiates the drivers based on what the board
> specifies.  There's not really any overlap between the two areas that I
> can see.
> 
> Otherwise could you explain in more detail where you see the code
> duplication coming from?

My point, though admittedly a minor one, was that the I2C device
or spi device could be initialized before the codec is probed.

That would be moving code from codec files into board files.
Better would be moving the code into a common soc directory file.

+#if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
+/*
+ * If the i2c layer weren't so broken, we could pass this kind of data
+ * around
+ */
+static int aic23_codec_probe(struct i2c_client *i2c,
+			     const struct i2c_device_id *i2c_id)
+{
+	struct snd_soc_device *socdev = aic23_socdev;
+	struct snd_soc_codec *codec = socdev->codec;
+	int ret;
+
+	if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+		return -EINVAL;
+
+
+	i2c_set_clientdata(i2c, codec);
+	codec->control_data = i2c;
+
....
+	ret = aic23_init(socdev);
+	if (ret < 0) {
+		printk(KERN_ERR "aic23: failed to initialise AIC23\n");
+		goto err;
+	}
... The above would move to codec probe

+	return ret;
+
+err:
+	kfree(codec);
+	kfree(i2c);
+	return ret;
+}
+static int __exit aic23_i2c_remove(struct i2c_client *i2c)
+{
+
+	put_device(&i2c->dev);
+	return 0;
+}
+
+static const struct i2c_device_id tlvaic23b_id[] = {
+	{"tlvaic23b", 0},
+	{}
+};
+
+MODULE_DEVICE_TABLE(i2c, tlvaic23b_id);
+
+static struct i2c_driver aic23b_i2c_driver = {
+	.driver = {
+		   .name = "tlvaic23b",
+		   },
+	.probe = aic23_codec_probe,
+	.remove = __exit_p(aic23_i2c_remove),
+	.id_table = tlvaic23b_id,
+};
+
+#endif

If this code could be generalized to be useable by most
codecs, I think the code would look better. This is the
"almost" duplication to which I'm referring.


Troy

  reply	other threads:[~2008-09-29 19:08 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-29  9:41 [PATCH 0/5] [RFC] ALSA ASoC Support for TLVaic23b audio codec Arun KS
2008-09-29 16:47 ` Troy Kisky
2008-09-29 16:54   ` Mark Brown
2008-09-29 17:05     ` Troy Kisky
2008-09-29 17:32       ` Mark Brown
2008-09-29 17:27     ` Troy Kisky
2008-09-29 17:39       ` Mark Brown
2008-09-29 17:58         ` Troy Kisky
2008-09-29 18:30           ` Mark Brown
2008-09-29 19:07             ` Troy Kisky [this message]
2008-09-29 19:50               ` Mark Brown
2008-09-29 21:15                 ` Troy Kisky
  -- strict thread matches above, loose matches on Subject: below --
2008-09-29  9:41 Arun KS

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=48E12789.50006@boundarydevices.com \
    --to=troy.kisky@boundarydevices.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=arunks@mistralsolutions.com \
    --cc=broonie@sirena.org.uk \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.