All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ASoC: introduce SOC_SINGLE_S8_TLV macro
@ 2012-01-08  1:53 Patrick Lai
  2012-01-08 20:23 ` Mark Brown
  0 siblings, 1 reply; 6+ messages in thread
From: Patrick Lai @ 2012-01-08  1:53 UTC (permalink / raw)
  To: broonie, lrg; +Cc: alsa-devel, Patrick Lai

ASoC currently has macro for double 8-bit signed TLV.
Enhance info,get,put callback functions of SOC_DOUBLE_S8_TLV
for single single 8-bit signed TLV.

Signed-off-by: Patrick Lai <plai@codeaurora.org>
---
 include/sound/soc.h  |   10 ++++++++++
 sound/soc/soc-core.c |   28 ++++++++++++++++++++--------
 2 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 11cfb59..558e9ff 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -55,6 +55,16 @@
 	.info = snd_soc_info_volsw, .get = snd_soc_get_volsw,\
 	.put = snd_soc_put_volsw, \
 	.private_value =  SOC_SINGLE_VALUE(reg, shift, max, invert) }
+#define SOC_SINGLE_S8_TLV(xname, xreg, xmin, xmax, tlv_array) \
+{	.iface  = SNDRV_CTL_ELEM_IFACE_MIXER, .name = (xname), \
+	.access = SNDRV_CTL_ELEM_ACCESS_TLV_READ | \
+		SNDRV_CTL_ELEM_ACCESS_READWRITE, \
+	.tlv.p  = (tlv_array), \
+	.info   = snd_soc_info_volsw_s8, .get = snd_soc_get_volsw_s8, \
+	.put    = snd_soc_put_volsw_s8, \
+	.private_value = (unsigned long)&(struct soc_mixer_control) \
+		{.reg = xreg, .min = xmin, .max = xmax, \
+		 .platform_max = xmax} }
 #define SOC_DOUBLE(xname, reg, shift_left, shift_right, max, invert) \
 {	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = (xname),\
 	.info = snd_soc_info_volsw, .get = snd_soc_get_volsw, \
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index a5d3685..3d1ba5f 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2399,13 +2399,15 @@ int snd_soc_info_volsw_s8(struct snd_kcontrol *kcontrol,
 		(struct soc_mixer_control *)kcontrol->private_value;
 	int platform_max;
 	int min = mc->min;
+	unsigned int shift = mc->shift;
+	unsigned int rshift = mc->rshift;
 
 	if (!mc->platform_max)
 		mc->platform_max = mc->max;
 	platform_max = mc->platform_max;
 
 	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
-	uinfo->count = 2;
+	uinfo->count = shift == rshift ? 1 : 2;
 	uinfo->value.integer.min = 0;
 	uinfo->value.integer.max = platform_max - min;
 	return 0;
@@ -2428,13 +2430,16 @@ int snd_soc_get_volsw_s8(struct snd_kcontrol *kcontrol,
 		(struct soc_mixer_control *)kcontrol->private_value;
 	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
 	unsigned int reg = mc->reg;
+	unsigned int shift = mc->shift;
+	unsigned int rshift = mc->rshift;
 	int min = mc->min;
 	int val = snd_soc_read(codec, reg);
 
 	ucontrol->value.integer.value[0] =
-		((signed char)(val & 0xff))-min;
-	ucontrol->value.integer.value[1] =
-		((signed char)((val >> 8) & 0xff))-min;
+		((signed char)((val >> shift) & 0xff))-min;
+	if (shift != rshift)
+		ucontrol->value.integer.value[1] =
+			((signed char)((val >> rshift) & 0xff))-min;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(snd_soc_get_volsw_s8);
@@ -2455,13 +2460,20 @@ int snd_soc_put_volsw_s8(struct snd_kcontrol *kcontrol,
 		(struct soc_mixer_control *)kcontrol->private_value;
 	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
 	unsigned int reg = mc->reg;
+	unsigned int shift = mc->shift;
+	unsigned int rshift = mc->rshift;
 	int min = mc->min;
-	unsigned int val;
+	unsigned int val, val2, val_mask;
 
-	val = (ucontrol->value.integer.value[0]+min) & 0xff;
-	val |= ((ucontrol->value.integer.value[1]+min) & 0xff) << 8;
+	val = ((ucontrol->value.integer.value[0]+min) & 0xff) << shift;
+	val_mask = 0xff << shift;
+	if (shift != rshift) {
+		val2 = (ucontrol->value.integer.value[1]+min) & 0xff;
+		val |= val2 << rshift;
+		val_mask |= 0xff << rshift;
+	}
 
-	return snd_soc_update_bits_locked(codec, reg, 0xffff, val);
+	return snd_soc_update_bits_locked(codec, reg, val_mask, val);
 }
 EXPORT_SYMBOL_GPL(snd_soc_put_volsw_s8);
 
-- 
1.7.3.3
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] ASoC: introduce SOC_SINGLE_S8_TLV macro
  2012-01-08  1:53 [PATCH] ASoC: introduce SOC_SINGLE_S8_TLV macro Patrick Lai
@ 2012-01-08 20:23 ` Mark Brown
  2012-01-09  5:52   ` Patrick Lai
  2012-01-09  8:21   ` Lars-Peter Clausen
  0 siblings, 2 replies; 6+ messages in thread
From: Mark Brown @ 2012-01-08 20:23 UTC (permalink / raw)
  To: Patrick Lai; +Cc: alsa-devel, lrg

On Sat, Jan 07, 2012 at 05:53:26PM -0800, Patrick Lai wrote:
> ASoC currently has macro for double 8-bit signed TLV.
> Enhance info,get,put callback functions of SOC_DOUBLE_S8_TLV
> for single single 8-bit signed TLV.

Actually now I look at this I'm not sure that the existing macro isn't
just misnamed:

| #define SOC_DOUBLE_S8_TLV(xname, xreg, xmin, xmax, tlv_array) \

Since no shifts are specified the signature is just the same as your
SOC_SINGLE_S8_TLV:

> +#define SOC_SINGLE_S8_TLV(xname, xreg, xmin, xmax, tlv_array) \

so it looks like what really ought to happen here is that the existing
macro gets renamed to the new macro and a new macro gets defined for
double values if one is needed.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] ASoC: introduce SOC_SINGLE_S8_TLV macro
  2012-01-08 20:23 ` Mark Brown
@ 2012-01-09  5:52   ` Patrick Lai
  2012-01-09  6:26     ` Mark Brown
  2012-01-09  8:21   ` Lars-Peter Clausen
  1 sibling, 1 reply; 6+ messages in thread
From: Patrick Lai @ 2012-01-09  5:52 UTC (permalink / raw)
  To: Mark Brown; +Cc: alsa-devel, lrg

> | #define SOC_DOUBLE_S8_TLV(xname, xreg, xmin, xmax, tlv_array) \
>
> Since no shifts are specified the signature is just the same as your
> SOC_SINGLE_S8_TLV:

Yes, it could have been made more generic with shift parameter.
Existing behavior is writing same value to most and least significant
bytes of 16-bit wide register.

>> +#define SOC_SINGLE_S8_TLV(xname, xreg, xmin, xmax, tlv_array) \
>
> so it looks like what really ought to happen here is that the existing
> macro gets renamed to the new macro and a new macro gets defined for
> double values if one is needed.

Is there backward compatibility concern? I see soc/codec/uda1380.c
already using SOC_DOUBLE_S8_TLV.

Thanks
Patrick

--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
---

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] ASoC: introduce SOC_SINGLE_S8_TLV macro
  2012-01-09  5:52   ` Patrick Lai
@ 2012-01-09  6:26     ` Mark Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2012-01-09  6:26 UTC (permalink / raw)
  To: Patrick Lai; +Cc: alsa-devel, lrg

On Sun, Jan 08, 2012 at 09:52:17PM -0800, Patrick Lai wrote:

> >so it looks like what really ought to happen here is that the existing
> >macro gets renamed to the new macro and a new macro gets defined for
> >double values if one is needed.

> Is there backward compatibility concern? I see soc/codec/uda1380.c
> already using SOC_DOUBLE_S8_TLV.

Obviously all existing users would need to be updated.  Out of tree
users will fail to compile which is fine, it's not going to fail
mysteriously at runtime or anything.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] ASoC: introduce SOC_SINGLE_S8_TLV macro
  2012-01-08 20:23 ` Mark Brown
  2012-01-09  5:52   ` Patrick Lai
@ 2012-01-09  8:21   ` Lars-Peter Clausen
  2012-01-09  8:28     ` Mark Brown
  1 sibling, 1 reply; 6+ messages in thread
From: Lars-Peter Clausen @ 2012-01-09  8:21 UTC (permalink / raw)
  To: Mark Brown; +Cc: Patrick Lai, alsa-devel, lrg

On 01/08/2012 09:23 PM, Mark Brown wrote:
> On Sat, Jan 07, 2012 at 05:53:26PM -0800, Patrick Lai wrote:
>> ASoC currently has macro for double 8-bit signed TLV.
>> Enhance info,get,put callback functions of SOC_DOUBLE_S8_TLV
>> for single single 8-bit signed TLV.
> 
> Actually now I look at this I'm not sure that the existing macro isn't
> just misnamed:
> 
> | #define SOC_DOUBLE_S8_TLV(xname, xreg, xmin, xmax, tlv_array) \
> 
> Since no shifts are specified the signature is just the same as your
> SOC_SINGLE_S8_TLV:
> 

There is a shift, but it is hard coded to 8 in snd_soc_get_volsw_s8, which
isn't considered by the patch currently. So either SOC_DOUBLE_S8_TLV should
initialize rshift to 8 or a shift and rshift parameter should be added. It
might also makes sense to add a shift parameter to SOC_SINGLE_S8_TLV as well.

- Lars

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] ASoC: introduce SOC_SINGLE_S8_TLV macro
  2012-01-09  8:21   ` Lars-Peter Clausen
@ 2012-01-09  8:28     ` Mark Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2012-01-09  8:28 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: Patrick Lai, alsa-devel, lrg

On Mon, Jan 09, 2012 at 09:21:19AM +0100, Lars-Peter Clausen wrote:
> On 01/08/2012 09:23 PM, Mark Brown wrote:

> > Since no shifts are specified the signature is just the same as your
> > SOC_SINGLE_S8_TLV:

> There is a shift, but it is hard coded to 8 in snd_soc_get_volsw_s8, which
> isn't considered by the patch currently. So either SOC_DOUBLE_S8_TLV should
> initialize rshift to 8 or a shift and rshift parameter should be added. It

The second option seems better here.

> might also makes sense to add a shift parameter to SOC_SINGLE_S8_TLV as well.

Probably.

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-01-09  8:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-08  1:53 [PATCH] ASoC: introduce SOC_SINGLE_S8_TLV macro Patrick Lai
2012-01-08 20:23 ` Mark Brown
2012-01-09  5:52   ` Patrick Lai
2012-01-09  6:26     ` Mark Brown
2012-01-09  8:21   ` Lars-Peter Clausen
2012-01-09  8:28     ` Mark Brown

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.