[alsa-devel] [PATCH] ASoC: introduce SOC_SINGLE_S8_TLV macro
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);
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.
| #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. ---
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.
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
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.
participants (3)
-
Lars-Peter Clausen
-
Mark Brown
-
Patrick Lai