[PATCH] ASoC: core: Pass kcontrol pointer to bytes tlv callbacks

[PATCH] ASoC: core: Pass kcontrol pointer to bytes tlv callbacks

[Ben Zhang]

The get/put callbacks need the kcontrol pointer to get context
information like snd_soc_codec and drvdata.

Signed-off-by: Ben Zhang <benzh@chromium.org>
---
 include/sound/soc.h | 6 ++++--
 sound/soc/soc-ops.c | 4 ++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index fcb312b..404265d 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1169,8 +1169,10 @@ struct soc_bytes {
 struct soc_bytes_ext {
 	int max;
 	/* used for TLV byte control */
-	int (*get)(unsigned int __user *bytes, unsigned int size);
-	int (*put)(const unsigned int __user *bytes, unsigned int size);
+	int (*get)(struct snd_kcontrol *kcontrol,
+		   unsigned int __user *bytes, unsigned int size);
+	int (*put)(struct snd_kcontrol *kcontrol,
+		   const unsigned int __user *bytes, unsigned int size);
 };
 
 /* multi register control */
diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c
index 100d92b..7f53da9 100644
--- a/sound/soc/soc-ops.c
+++ b/sound/soc/soc-ops.c
@@ -751,11 +751,11 @@ int snd_soc_bytes_tlv_callback(struct snd_kcontrol *kcontrol, int op_flag,
 	switch (op_flag) {
 	case SNDRV_CTL_TLV_OP_READ:
 		if (params->get)
-			ret = params->get(tlv, count);
+			ret = params->get(kcontrol, tlv, count);
 		break;
 	case SNDRV_CTL_TLV_OP_WRITE:
 		if (params->put)
-			ret = params->put(tlv, count);
+			ret = params->put(kcontrol, tlv, count);
 		break;
 	}
 	return ret;
-- 
2.2.0.rc0.207.ga3a616c

Re: [PATCH] ASoC: core: Pass kcontrol pointer to bytes tlv callbacks

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 482 bytes --]

On Tue, Jun 02, 2015 at 03:24:03PM -0700, Ben Zhang wrote:

> -	int (*get)(unsigned int __user *bytes, unsigned int size);
> -	int (*put)(const unsigned int __user *bytes, unsigned int size);
> +	int (*get)(struct snd_kcontrol *kcontrol,
> +		   unsigned int __user *bytes, unsigned int size);
> +	int (*put)(struct snd_kcontrol *kcontrol,
> +		   const unsigned int __user *bytes, unsigned int size);

This doesn't update any users and we do have some like the Haswell PCM
driver.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] ASoC: core: Pass kcontrol pointer to bytes tlv callbacks

[Ben Zhang]

On Wed, Jun 3, 2015 at 4:59 AM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Jun 02, 2015 at 03:24:03PM -0700, Ben Zhang wrote:
>
>> -     int (*get)(unsigned int __user *bytes, unsigned int size);
>> -     int (*put)(const unsigned int __user *bytes, unsigned int size);
>> +     int (*get)(struct snd_kcontrol *kcontrol,
>> +                unsigned int __user *bytes, unsigned int size);
>> +     int (*put)(struct snd_kcontrol *kcontrol,
>> +                const unsigned int __user *bytes, unsigned int size);
>
> This doesn't update any users and we do have some like the Haswell PCM
> driver.

Hi Mark,

I couldn't find any existing users of the callbacks. It looks like the
modified callbacks are used by SND_SOC_BYTES_TLV only, and there is no
reference to SND_SOC_BYTES_TLV in the kernel.

The Haswell PCM driver uses SND_SOC_BYTES_EXT which doesn't use the
callbacks in struct soc_bytes_ext. The xhandler_get/put are assigned
to snd_kcontrol_new.get/put instead.

Ben

Re: [PATCH] ASoC: core: Pass kcontrol pointer to bytes tlv callbacks

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1327 bytes --]

On Wed, Jun 03, 2015 at 02:10:28PM -0700, Ben Zhang wrote:
> On Wed, Jun 3, 2015 at 4:59 AM, Mark Brown <broonie@kernel.org> wrote:
> > On Tue, Jun 02, 2015 at 03:24:03PM -0700, Ben Zhang wrote:

> >> -     int (*get)(unsigned int __user *bytes, unsigned int size);
> >> -     int (*put)(const unsigned int __user *bytes, unsigned int size);
> >> +     int (*get)(struct snd_kcontrol *kcontrol,
> >> +                unsigned int __user *bytes, unsigned int size);
> >> +     int (*put)(struct snd_kcontrol *kcontrol,
> >> +                const unsigned int __user *bytes, unsigned int size);

> > This doesn't update any users and we do have some like the Haswell PCM
> > driver.

> I couldn't find any existing users of the callbacks. It looks like the
> modified callbacks are used by SND_SOC_BYTES_TLV only, and there is no
> reference to SND_SOC_BYTES_TLV in the kernel.

> The Haswell PCM driver uses SND_SOC_BYTES_EXT which doesn't use the
> callbacks in struct soc_bytes_ext. The xhandler_get/put are assigned
> to snd_kcontrol_new.get/put instead.

This then begs the question why the fix isn't for the TLV version to use
the same callbacks we're currently using?  It's not obvious looking at
the definitions of the controls why we would want them to be using
separate ones and TLV would normally not affect the I/O.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] ASoC: core: Pass kcontrol pointer to bytes tlv callbacks

[Vinod Koul]

On Tue, Jun 02, 2015 at 03:24:03PM -0700, Ben Zhang wrote:
> The get/put callbacks need the kcontrol pointer to get context
> information like snd_soc_codec and drvdata.
> 
> Signed-off-by: Ben Zhang <benzh@chromium.org>
I did have this change in my internal tree as well and is required, thanks
for sending, I need this is SKL driver..

Reviewed-by: Vinod Koul <vinod.koul@intel.com>

-- 
~Vinod

> ---
>  include/sound/soc.h | 6 ++++--
>  sound/soc/soc-ops.c | 4 ++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index fcb312b..404265d 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -1169,8 +1169,10 @@ struct soc_bytes {
>  struct soc_bytes_ext {
>  	int max;
>  	/* used for TLV byte control */
> -	int (*get)(unsigned int __user *bytes, unsigned int size);
> -	int (*put)(const unsigned int __user *bytes, unsigned int size);
> +	int (*get)(struct snd_kcontrol *kcontrol,
> +		   unsigned int __user *bytes, unsigned int size);
> +	int (*put)(struct snd_kcontrol *kcontrol,
> +		   const unsigned int __user *bytes, unsigned int size);
>  };
>  
>  /* multi register control */
> diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c
> index 100d92b..7f53da9 100644
> --- a/sound/soc/soc-ops.c
> +++ b/sound/soc/soc-ops.c
> @@ -751,11 +751,11 @@ int snd_soc_bytes_tlv_callback(struct snd_kcontrol *kcontrol, int op_flag,
>  	switch (op_flag) {
>  	case SNDRV_CTL_TLV_OP_READ:
>  		if (params->get)
> -			ret = params->get(tlv, count);
> +			ret = params->get(kcontrol, tlv, count);
>  		break;
>  	case SNDRV_CTL_TLV_OP_WRITE:
>  		if (params->put)
> -			ret = params->put(tlv, count);
> +			ret = params->put(kcontrol, tlv, count);
>  		break;
>  	}
>  	return ret;
> -- 
> 2.2.0.rc0.207.ga3a616c
> 

--