diff mbox

[04/13] ASoC: Move IO functions to soc-io.c

Message ID 1395129736-11938-5-git-send-email-lars@metafoo.de (mailing list archive)
State Accepted
Commit 96241c83293de346037b9a85e321f52ace210926
Headers show

Commit Message

Lars-Peter Clausen March 18, 2014, 8:02 a.m. UTC 
soc-core.c is getting quite crowded. Move all IO related functions that are
still in soc-core.c to soc-io.c

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/soc-core.c | 144 ---------------------------------------------------
 sound/soc/soc-io.c   | 144 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 144 insertions(+), 144 deletions(-)

Comments

Mark Brown March 19, 2014, 11:10 a.m. UTC | #1 
On Tue, Mar 18, 2014 at 09:02:07AM +0100, Lars-Peter Clausen wrote:
> soc-core.c is getting quite crowded. Move all IO related functions that are
> still in soc-core.c to soc-io.c

No, we're trying to make soc-io smaller!  Besides, if this stuff is
getting cleaned up then:

> -unsigned int snd_soc_read(struct snd_soc_codec *codec, unsigned int reg)
> -{
> -	unsigned int ret;
> -
> -	ret = codec->read(codec, reg);
> -	dev_dbg(codec->dev, "read %x => %x\n", reg, ret);
> -	trace_snd_soc_reg_read(codec, reg, ret);
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL_GPL(snd_soc_read);

...making small things like this static inlines in a header is probably a
better move (we can probably drop the trace - I don't think anything in
active development doesn't use regmap and it's a carrot to push people).
Lars-Peter Clausen March 19, 2014, 11:47 a.m. UTC | #2 
On 03/19/2014 12:10 PM, Mark Brown wrote:
> On Tue, Mar 18, 2014 at 09:02:07AM +0100, Lars-Peter Clausen wrote:
>> soc-core.c is getting quite crowded. Move all IO related functions that are
>> still in soc-core.c to soc-io.c
>
> No, we're trying to make soc-io smaller!  Besides, if this stuff is
> getting cleaned up then:
>

We are trying to remove the ASoC level IO abstraction layer. In my opinion 
keeping everything related to this in one file, rather than hiding it among 
5k lines of other code, makes it clear what is still left to do.

>> -unsigned int snd_soc_read(struct snd_soc_codec *codec, unsigned int reg)
>> -{
>> -	unsigned int ret;
>> -
>> -	ret = codec->read(codec, reg);
>> -	dev_dbg(codec->dev, "read %x => %x\n", reg, ret);
>> -	trace_snd_soc_reg_read(codec, reg, ret);
>> -
>> -	return ret;
>> -}
>> -EXPORT_SYMBOL_GPL(snd_soc_read);
>
> ...making small things like this static inlines in a header is probably a
> better move (we can probably drop the trace - I don't think anything in
> active development doesn't use regmap and it's a carrot to push people).

Ok, if we don't care of about the traces I'll remove them and make the 
function static inlines.

- Lars
Mark Brown March 19, 2014, 11:53 a.m. UTC | #3 
On Wed, Mar 19, 2014 at 12:47:59PM +0100, Lars-Peter Clausen wrote:
> On 03/19/2014 12:10 PM, Mark Brown wrote:

> >No, we're trying to make soc-io smaller!  Besides, if this stuff is
> >getting cleaned up then:

> We are trying to remove the ASoC level IO abstraction layer. In my opinion
> keeping everything related to this in one file, rather than hiding it among
> 5k lines of other code, makes it clear what is still left to do.

Like I've said before it is not obvious to me that removing the wrappers
is a win - yes, things can obtain their regmap but if enough things need
to do it then having a convenience wrapper which does the lookup at
least doesn't hurt.
Lars-Peter Clausen March 19, 2014, 11:57 a.m. UTC | #4 
On 03/19/2014 12:53 PM, Mark Brown wrote:
> On Wed, Mar 19, 2014 at 12:47:59PM +0100, Lars-Peter Clausen wrote:
>> On 03/19/2014 12:10 PM, Mark Brown wrote:
>
>>> No, we're trying to make soc-io smaller!  Besides, if this stuff is
>>> getting cleaned up then:
>
>> We are trying to remove the ASoC level IO abstraction layer. In my opinion
>> keeping everything related to this in one file, rather than hiding it among
>> 5k lines of other code, makes it clear what is still left to do.
>
> Like I've said before it is not obvious to me that removing the wrappers
> is a win - yes, things can obtain their regmap but if enough things need
> to do it then having a convenience wrapper which does the lookup at
> least doesn't hurt.
>

It's more about things like snd_soc_update_bits. The wrappers will stay, but 
can as you said be made static inline functions and moved to the header.
Mark Brown March 19, 2014, 12:01 p.m. UTC | #5 
On Wed, Mar 19, 2014 at 12:57:57PM +0100, Lars-Peter Clausen wrote:
> On 03/19/2014 12:53 PM, Mark Brown wrote:

> >Like I've said before it is not obvious to me that removing the wrappers
> >is a win - yes, things can obtain their regmap but if enough things need
> >to do it then having a convenience wrapper which does the lookup at
> >least doesn't hurt.

> It's more about things like snd_soc_update_bits. The wrappers will stay, but
> can as you said be made static inline functions and moved to the header.

I'm confused - that's what I said isn't it?
Lars-Peter Clausen March 19, 2014, 12:11 p.m. UTC | #6 
On 03/19/2014 01:01 PM, Mark Brown wrote:
> On Wed, Mar 19, 2014 at 12:57:57PM +0100, Lars-Peter Clausen wrote:
>> On 03/19/2014 12:53 PM, Mark Brown wrote:
>
>>> Like I've said before it is not obvious to me that removing the wrappers
>>> is a win - yes, things can obtain their regmap but if enough things need
>>> to do it then having a convenience wrapper which does the lookup at
>>> least doesn't hurt.
>
>> It's more about things like snd_soc_update_bits. The wrappers will stay, but
>> can as you said be made static inline functions and moved to the header.
>
> I'm confused - that's what I said isn't it?

I think we are on the same page in regards to were we want to go.

I never said that I wanted to remove the wrappers, they hide the function 
from the implementation which is good in two ways, the user doesn't have to 
deal with the details implementation and secondly we can update the 
implementation without having to update the users.

What this series does, first moves everything related to ASoC level IO to 
soc-io.c and then starts cleaning it up. For now we still have to maintain 
some of the ASoC level IO implementation like snd_soc_update_bits() since 
not all drivers are using regmap yet. Once all have been converted 
snd_soc_component_write(...) boils down to regmap_write(component->regmap, 
...), then we can move it to a header and make it static inline and remove 
soc-io.c.
diff mbox

Patch

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 0a39031..b66ccce 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2039,38 +2039,6 @@  int snd_soc_codec_writable_register(struct snd_soc_codec *codec,
 }
 EXPORT_SYMBOL_GPL(snd_soc_codec_writable_register);
 
-int snd_soc_platform_read(struct snd_soc_platform *platform,
-					unsigned int reg)
-{
-	unsigned int ret;
-
-	if (!platform->driver->read) {
-		dev_err(platform->dev, "ASoC: platform has no read back\n");
-		return -1;
-	}
-
-	ret = platform->driver->read(platform, reg);
-	dev_dbg(platform->dev, "read %x => %x\n", reg, ret);
-	trace_snd_soc_preg_read(platform, reg, ret);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(snd_soc_platform_read);
-
-int snd_soc_platform_write(struct snd_soc_platform *platform,
-					 unsigned int reg, unsigned int val)
-{
-	if (!platform->driver->write) {
-		dev_err(platform->dev, "ASoC: platform has no write back\n");
-		return -1;
-	}
-
-	dev_dbg(platform->dev, "write %x = %x\n", reg, val);
-	trace_snd_soc_preg_write(platform, reg, val);
-	return platform->driver->write(platform, reg, val);
-}
-EXPORT_SYMBOL_GPL(snd_soc_platform_write);
-
 /**
  * snd_soc_new_ac97_codec - initailise AC97 device
  * @codec: audio codec
@@ -2289,118 +2257,6 @@  void snd_soc_free_ac97_codec(struct snd_soc_codec *codec)
 }
 EXPORT_SYMBOL_GPL(snd_soc_free_ac97_codec);
 
-unsigned int snd_soc_read(struct snd_soc_codec *codec, unsigned int reg)
-{
-	unsigned int ret;
-
-	ret = codec->read(codec, reg);
-	dev_dbg(codec->dev, "read %x => %x\n", reg, ret);
-	trace_snd_soc_reg_read(codec, reg, ret);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(snd_soc_read);
-
-unsigned int snd_soc_write(struct snd_soc_codec *codec,
-			   unsigned int reg, unsigned int val)
-{
-	dev_dbg(codec->dev, "write %x = %x\n", reg, val);
-	trace_snd_soc_reg_write(codec, reg, val);
-	return codec->write(codec, reg, val);
-}
-EXPORT_SYMBOL_GPL(snd_soc_write);
-
-/**
- * snd_soc_update_bits - update codec register bits
- * @codec: audio codec
- * @reg: codec register
- * @mask: register mask
- * @value: new value
- *
- * Writes new register value.
- *
- * Returns 1 for change, 0 for no change, or negative error code.
- */
-int snd_soc_update_bits(struct snd_soc_codec *codec, unsigned short reg,
-				unsigned int mask, unsigned int value)
-{
-	bool change;
-	unsigned int old, new;
-	int ret;
-
-	if (codec->using_regmap) {
-		ret = regmap_update_bits_check(codec->control_data, reg,
-					       mask, value, &change);
-	} else {
-		ret = snd_soc_read(codec, reg);
-		if (ret < 0)
-			return ret;
-
-		old = ret;
-		new = (old & ~mask) | (value & mask);
-		change = old != new;
-		if (change)
-			ret = snd_soc_write(codec, reg, new);
-	}
-
-	if (ret < 0)
-		return ret;
-
-	return change;
-}
-EXPORT_SYMBOL_GPL(snd_soc_update_bits);
-
-/**
- * snd_soc_update_bits_locked - update codec register bits
- * @codec: audio codec
- * @reg: codec register
- * @mask: register mask
- * @value: new value
- *
- * Writes new register value, and takes the codec mutex.
- *
- * Returns 1 for change else 0.
- */
-int snd_soc_update_bits_locked(struct snd_soc_codec *codec,
-			       unsigned short reg, unsigned int mask,
-			       unsigned int value)
-{
-	int change;
-
-	mutex_lock(&codec->mutex);
-	change = snd_soc_update_bits(codec, reg, mask, value);
-	mutex_unlock(&codec->mutex);
-
-	return change;
-}
-EXPORT_SYMBOL_GPL(snd_soc_update_bits_locked);
-
-/**
- * snd_soc_test_bits - test register for change
- * @codec: audio codec
- * @reg: codec register
- * @mask: register mask
- * @value: new value
- *
- * Tests a register with a new value and checks if the new value is
- * different from the old value.
- *
- * Returns 1 for change else 0.
- */
-int snd_soc_test_bits(struct snd_soc_codec *codec, unsigned short reg,
-				unsigned int mask, unsigned int value)
-{
-	int change;
-	unsigned int old, new;
-
-	old = snd_soc_read(codec, reg);
-	new = (old & ~mask) | value;
-	change = old != new;
-
-	return change;
-}
-EXPORT_SYMBOL_GPL(snd_soc_test_bits);
-
 /**
  * snd_soc_cnew - create new control
  * @_template: control template
diff --git a/sound/soc/soc-io.c b/sound/soc/soc-io.c
index 8aa0869..de1e496 100644
--- a/sound/soc/soc-io.c
+++ b/sound/soc/soc-io.c
@@ -19,6 +19,150 @@ 
 
 #include <trace/events/asoc.h>
 
+unsigned int snd_soc_read(struct snd_soc_codec *codec, unsigned int reg)
+{
+	unsigned int ret;
+
+	ret = codec->read(codec, reg);
+	dev_dbg(codec->dev, "read %x => %x\n", reg, ret);
+	trace_snd_soc_reg_read(codec, reg, ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(snd_soc_read);
+
+unsigned int snd_soc_write(struct snd_soc_codec *codec,
+			   unsigned int reg, unsigned int val)
+{
+	dev_dbg(codec->dev, "write %x = %x\n", reg, val);
+	trace_snd_soc_reg_write(codec, reg, val);
+	return codec->write(codec, reg, val);
+}
+EXPORT_SYMBOL_GPL(snd_soc_write);
+
+/**
+ * snd_soc_update_bits - update codec register bits
+ * @codec: audio codec
+ * @reg: codec register
+ * @mask: register mask
+ * @value: new value
+ *
+ * Writes new register value.
+ *
+ * Returns 1 for change, 0 for no change, or negative error code.
+ */
+int snd_soc_update_bits(struct snd_soc_codec *codec, unsigned short reg,
+				unsigned int mask, unsigned int value)
+{
+	bool change;
+	unsigned int old, new;
+	int ret;
+
+	if (codec->using_regmap) {
+		ret = regmap_update_bits_check(codec->control_data, reg,
+					       mask, value, &change);
+	} else {
+		ret = snd_soc_read(codec, reg);
+		if (ret < 0)
+			return ret;
+
+		old = ret;
+		new = (old & ~mask) | (value & mask);
+		change = old != new;
+		if (change)
+			ret = snd_soc_write(codec, reg, new);
+	}
+
+	if (ret < 0)
+		return ret;
+
+	return change;
+}
+EXPORT_SYMBOL_GPL(snd_soc_update_bits);
+
+/**
+ * snd_soc_update_bits_locked - update codec register bits
+ * @codec: audio codec
+ * @reg: codec register
+ * @mask: register mask
+ * @value: new value
+ *
+ * Writes new register value, and takes the codec mutex.
+ *
+ * Returns 1 for change else 0.
+ */
+int snd_soc_update_bits_locked(struct snd_soc_codec *codec,
+			       unsigned short reg, unsigned int mask,
+			       unsigned int value)
+{
+	int change;
+
+	mutex_lock(&codec->mutex);
+	change = snd_soc_update_bits(codec, reg, mask, value);
+	mutex_unlock(&codec->mutex);
+
+	return change;
+}
+EXPORT_SYMBOL_GPL(snd_soc_update_bits_locked);
+
+/**
+ * snd_soc_test_bits - test register for change
+ * @codec: audio codec
+ * @reg: codec register
+ * @mask: register mask
+ * @value: new value
+ *
+ * Tests a register with a new value and checks if the new value is
+ * different from the old value.
+ *
+ * Returns 1 for change else 0.
+ */
+int snd_soc_test_bits(struct snd_soc_codec *codec, unsigned short reg,
+				unsigned int mask, unsigned int value)
+{
+	int change;
+	unsigned int old, new;
+
+	old = snd_soc_read(codec, reg);
+	new = (old & ~mask) | value;
+	change = old != new;
+
+	return change;
+}
+EXPORT_SYMBOL_GPL(snd_soc_test_bits);
+
+int snd_soc_platform_read(struct snd_soc_platform *platform,
+					unsigned int reg)
+{
+	unsigned int ret;
+
+	if (!platform->driver->read) {
+		dev_err(platform->dev, "ASoC: platform has no read back\n");
+		return -1;
+	}
+
+	ret = platform->driver->read(platform, reg);
+	dev_dbg(platform->dev, "read %x => %x\n", reg, ret);
+	trace_snd_soc_preg_read(platform, reg, ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(snd_soc_platform_read);
+
+int snd_soc_platform_write(struct snd_soc_platform *platform,
+					 unsigned int reg, unsigned int val)
+{
+	if (!platform->driver->write) {
+		dev_err(platform->dev, "ASoC: platform has no write back\n");
+		return -1;
+	}
+
+	dev_dbg(platform->dev, "write %x = %x\n", reg, val);
+	trace_snd_soc_preg_write(platform, reg, val);
+	return platform->driver->write(platform, reg, val);
+}
+EXPORT_SYMBOL_GPL(snd_soc_platform_write);
+
 #ifdef CONFIG_REGMAP
 static int hw_write(struct snd_soc_codec *codec, unsigned int reg,
 		    unsigned int value)