diff mbox

[1/2] ASoC: add api for dapm kcontrol configiuration

Message ID 1401106136-3745-1-git-send-email-vinod.koul@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vinod Koul May 26, 2014, 12:08 p.m. UTC 
From: Subhransu S. Prusty <subhransu.s.prusty@intel.com>

For DSPs we need to set and get the value for snd_kcontrol. This is currently
done by dapm_kcontrol_set/get_value, so create a wrapper
snd_soc_dapm_kcontrol_get/set_value APIs to let drivers use this

Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 include/sound/soc-dapm.h |    4 ++++
 sound/soc/soc-dapm.c     |   13 +++++++++++++
 2 files changed, 17 insertions(+), 0 deletions(-)

Comments

Lars-Peter Clausen May 26, 2014, 1:26 p.m. UTC | #1 
On 05/26/2014 02:08 PM, Vinod Koul wrote:
> From: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
>
> For DSPs we need to set and get the value for snd_kcontrol. This is currently
> done by dapm_kcontrol_set/get_value, so create a wrapper
> snd_soc_dapm_kcontrol_get/set_value APIs to let drivers use this

Can you go a bit more into detail how you intend to use this?

> [...]
> +unsigned int snd_soc_dapm_kcontrol_get_value(
> +	const struct snd_kcontrol *kcontrol)
> +{
> +	dapm_kcontrol_get_value(kcontrol);

The compiler should have complained about this.

> +}
> +EXPORT_SYMBOL_GPL(snd_soc_dapm_kcontrol_get_value);

I'm also not sure how much sense it makes to have a single line wrapper 
function for exporting things? Why not just rename the original function and 
then export it?
Mark Brown May 26, 2014, 3:05 p.m. UTC | #2 
On Mon, May 26, 2014 at 05:38:55PM +0530, Vinod Koul wrote:
> From: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> 
> For DSPs we need to set and get the value for snd_kcontrol. This is currently
> done by dapm_kcontrol_set/get_value, so create a wrapper
> snd_soc_dapm_kcontrol_get/set_value APIs to let drivers use this

Can you provide any more detail on this than just simply stating that
this is needed for DSPs - why do DSPs need it, what are they going to do
with it?

>  }
> +unsigned int snd_soc_dapm_kcontrol_get_value(

Missing blank line.

> +	const struct snd_kcontrol *kcontrol)
> +{
> +	dapm_kcontrol_get_value(kcontrol);
> +}
> +EXPORT_SYMBOL_GPL(snd_soc_dapm_kcontrol_get_value);

This function has a return value but no return statement, I'm surprised
the compiler didn't tell you this.  To be honest I'm not sure why
dapm_kcontrol_get_value() wasn't just exported, the wrapper isn't adding
much and the function name is already so long that snd_soc_ isn't going
to hurt.
Vinod Koul May 26, 2014, 4:18 p.m. UTC | #3 
On Mon, May 26, 2014 at 04:05:07PM +0100, Mark Brown wrote:
> On Mon, May 26, 2014 at 05:38:55PM +0530, Vinod Koul wrote:
> > From: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> > 
> > For DSPs we need to set and get the value for snd_kcontrol. This is currently
> > done by dapm_kcontrol_set/get_value, so create a wrapper
> > snd_soc_dapm_kcontrol_get/set_value APIs to let drivers use this
> 
> Can you provide any more detail on this than just simply stating that
> this is needed for DSPs - why do DSPs need it, what are they going to do
> with it?
Well,... planning to provide a patch too :)

For our DSPs we have mixers and we need to send messages to DSP on mixer enable
and disable by DAPM. So this fn helps us to get the value of widget and pass on
to the DSP.

The usage will be in my comming series for our DSP model using DAPM and DPCM.

> 
> >  }
> > +unsigned int snd_soc_dapm_kcontrol_get_value(
> 
> Missing blank line.
> 
> > +	const struct snd_kcontrol *kcontrol)
> > +{
> > +	dapm_kcontrol_get_value(kcontrol);
> > +}
> > +EXPORT_SYMBOL_GPL(snd_soc_dapm_kcontrol_get_value);
> 
> This function has a return value but no return statement, I'm surprised
> the compiler didn't tell you this.  To be honest I'm not sure why
> dapm_kcontrol_get_value() wasn't just exported, the wrapper isn't adding
> much and the function name is already so long that snd_soc_ isn't going
> to hurt.
Looks like patch got mangled, I will fix it up.

And agree to both your and Lar's comment that we dont need a wrapper so will
export the current funtion.

Thanks
diff mbox

Patch

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index 75020f5..9e826e9 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -368,6 +368,10 @@  int dapm_regulator_event(struct snd_soc_dapm_widget *w,
 			 struct snd_kcontrol *kcontrol, int event);
 int dapm_clock_event(struct snd_soc_dapm_widget *w,
 			 struct snd_kcontrol *kcontrol, int event);
+bool snd_soc_dapm_kcontrol_set_value(
+		const struct snd_kcontrol *kcontrol, unsigned int value);
+unsigned int snd_soc_dapm_kcontrol_get_value(
+		const struct snd_kcontrol *kcontrol);
 
 /* dapm controls */
 int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol,
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 98c1dc6..b7dfa1a 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -333,6 +333,12 @@  static unsigned int dapm_kcontrol_get_value(const struct snd_kcontrol *kcontrol)
 
 	return data->value;
 }
+unsigned int snd_soc_dapm_kcontrol_get_value(
+	const struct snd_kcontrol *kcontrol)
+{
+	dapm_kcontrol_get_value(kcontrol);
+}
+EXPORT_SYMBOL_GPL(snd_soc_dapm_kcontrol_get_value);
 
 static bool dapm_kcontrol_set_value(const struct snd_kcontrol *kcontrol,
 	unsigned int value)
@@ -350,6 +356,13 @@  static bool dapm_kcontrol_set_value(const struct snd_kcontrol *kcontrol,
 	return true;
 }
 
+bool snd_soc_dapm_kcontrol_set_value(const struct snd_kcontrol *kcontrol,
+	unsigned int value)
+{
+	return dapm_kcontrol_set_value(kcontrol, value);
+}
+EXPORT_SYMBOL_GPL(snd_soc_dapm_kcontrol_set_value);
+
 /**
  * snd_soc_dapm_kcontrol_codec() - Returns the codec associated to a kcontrol
  * @kcontrol: The kcontrol