[alsa-devel] [PATCH v4] ASoC: rt5645: Add the HW EQ for the customized speaker output of Google Celes

[Mark Brown]

On Wed, Oct 14, 2015 at 02:15:00PM +0800, Oder Chiou wrote:
> Signed-off-by: Oder Chiou <oder_chiou at realtek.com>

The userspace interface is now what I'd expec tbut I'm still quite
confused by this.  Why is this not a normal bytes control?  There's
something going on with private data here but I'm not sure what it's
supposed to do over writing to the device - a changelog might've
helped...

> +static int rt5645_hweq_get(struct snd_kcontrol *kcontrol,
> +			struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> +	struct rt5645_priv *rt5645 = snd_soc_component_get_drvdata(component);
> +	char *p = ucontrol->value.bytes.data;
> +
> +	if (rt5645->pdata.hweq)
> +		memcpy(p, rt5645->eq_param,
> +			RT5645_HWEQ_NUM * sizeof(struct rt5645_eq_param_s));
> +
> +	return 0;

There's a validation function below - we should be using it when the
user supplies data so they can tell if their settings took effect.

> +static struct rt5645_platform_data celes_platform_data = {
> +	.dmic1_data_pin = RT5645_DMIC1_DISABLE,
> +	.dmic2_data_pin = RT5645_DMIC_DATA_IN2P,
> +	.jd_mode = 3,
> +	.hweq = true,
> +};

Why is the hweq setting part of platform data (and why is the platform
data for a specific system being set as part of this patch)?  This is
just a setting that can be set, there's nothing system specific about it
and it's not like we're even passing in a system specific tuning here.

Please, one change per patch and describe changes in the changelog.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20151016/ec23cdbc/attachment.sig>