ASoC: pass snd_soc_jack_gpio to jack_status_check callback
diff mbox

Message ID 1392528972-9440-1-git-send-email-xiaoxiang@xiaomi.com
State New, archived
Delegated to: Mark Brown
Headers show

Commit Message

Xiang Xiao Feb. 16, 2014, 5:36 a.m. UTC
so the implementation could get the context data

Change-Id: I5542613ccf9881deb672e71d0f0c4f603ad761fd
Signed-off-by: xiangxiao <xiaoxiang@xiaomi.com>
---
 include/sound/soc.h  | 2 +-
 sound/soc/soc-jack.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Mark Brown Feb. 18, 2014, 1:16 a.m. UTC | #1
On Sun, Feb 16, 2014 at 01:36:12PM +0800, xiangxiao wrote:
> so the implementation could get the context data
> 
> Change-Id: I5542613ccf9881deb672e71d0f0c4f603ad761fd

This shouldn't be on upstream patches.

> Signed-off-by: xiangxiao <xiaoxiang@xiaomi.com>

> -	int (*jack_status_check)(void);
> +	int (*jack_status_check)(struct snd_soc_jack_gpio *gpio);

Why is the callback using details of the implementation of
snd_soc_jack_gpio?  Perhaps there's a good reason, I'm not sure and
you've not included any implementation.  I'd have expected to see the
jack able to provide its own context here?
Mark Brown Feb. 22, 2014, 3:21 a.m. UTC | #2
On Wed, Feb 19, 2014 at 02:20:07PM +0000, ?? wrote:
> The new parameter isn't used to get the internal field of snd_soc_jack_gpio.
> the usage is very like work_struct, here is the demo code:
> struct xxx_priv {
> 	truct snd_soc_jack_gpio gpio;
> };
> 
> static int xxx_ jack_status_check(struct snd_soc_jack_gpio *gpio)
> {
> 	struct xxx_priv *xxx = container_of(gpio, struct xxx_priv, gpio);
> 	......
> }
> Without this parameter, codec driver has to save xxx_priv as a global variable.

OK, this makes sense as a reason for passing the argument in however how
about instead of passing in the structure adding a void * to it which
gets passed as the argument instead?  The user can then assign it to
point to the data they want directly without it looking like they should
be using the private data for the core code.
Xiang Xiao Feb. 23, 2014, 6:04 a.m. UTC | #3
Sure, I will update the change per your suggestion.

-----????-----
???: Mark Brown [mailto:broonie@kernel.org] 
????: 2014?2?22? 11:22
???: ??
??: lgirdwood@gmail.com; alsa-devel@alsa-project.org
??: Re: ??: [PATCH] ASoC: pass snd_soc_jack_gpio to jack_status_check callback

On Wed, Feb 19, 2014 at 02:20:07PM +0000, ?? wrote:
> The new parameter isn't used to get the internal field of snd_soc_jack_gpio.

> the usage is very like work_struct, here is the demo code:

> struct xxx_priv {

> 	truct snd_soc_jack_gpio gpio;

> };

> 

> static int xxx_ jack_status_check(struct snd_soc_jack_gpio *gpio) {

> 	struct xxx_priv *xxx = container_of(gpio, struct xxx_priv, gpio);

> 	......

> }

> Without this parameter, codec driver has to save xxx_priv as a global variable.


OK, this makes sense as a reason for passing the argument in however how about instead of passing in the structure adding a void * to it which gets passed as the argument instead?  The user can then assign it to point to the data they want directly without it looking like they should be using the private data for the core code.

Patch
diff mbox

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 21038e0..e287462 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -613,7 +613,7 @@  struct snd_soc_jack_gpio {
 	struct snd_soc_jack *jack;
 	struct delayed_work work;
 
-	int (*jack_status_check)(void);
+	int (*jack_status_check)(struct snd_soc_jack_gpio *gpio);
 };
 
 struct snd_soc_jack {
diff --git a/sound/soc/soc-jack.c b/sound/soc/soc-jack.c
index b12fce6..6028af5 100644
--- a/sound/soc/soc-jack.c
+++ b/sound/soc/soc-jack.c
@@ -250,7 +250,7 @@  static void snd_soc_jack_gpio_detect(struct snd_soc_jack_gpio *gpio)
 		report = 0;
 
 	if (gpio->jack_status_check)
-		report = gpio->jack_status_check();
+		report = gpio->jack_status_check(gpio);
 
 	snd_soc_jack_report(jack, report, gpio->report);
 }