diff mbox

[v7,3/7] ALSA: jack: extend snd_jack_new to support phantom jack

Message ID 1429603545-21063-4-git-send-email-yang.jie@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jie, Yang April 21, 2015, 8:05 a.m. UTC
For phantom jack, we won't create real jack device, but jack kctl
may be needed.

Here, we extend snd_jack_new() to support phantom jack creating:
pass in a bool param for [non-]phantom flag, and a non-Null param
(struct snd_jack_kctl **) to indicate that we need create kctl
at this jack creating stage.

We can also add kctl to a jack after the jack is created.

This make the integrating the existing HDA jack kctl and soc jack
kctl possible.

Signed-off-by: Jie Yang <yang.jie@intel.com>
---
 include/sound/jack.h            |  4 +--
 sound/core/jack.c               | 56 +++++++++++++++++++++++++----------------
 sound/pci/hda/hda_jack.c        |  2 +-
 sound/pci/oxygen/xonar_wm87x6.c |  2 +-
 sound/soc/soc-jack.c            |  2 +-
 5 files changed, 39 insertions(+), 27 deletions(-)

Comments

Takashi Iwai April 21, 2015, 9:57 a.m. UTC | #1
At Tue, 21 Apr 2015 16:05:41 +0800,
Jie Yang wrote:
> 
> For phantom jack, we won't create real jack device, but jack kctl
> may be needed.
> 
> Here, we extend snd_jack_new() to support phantom jack creating:
> pass in a bool param for [non-]phantom flag, and a non-Null param
> (struct snd_jack_kctl **) to indicate that we need create kctl
> at this jack creating stage.
> 
> We can also add kctl to a jack after the jack is created.
> 
> This make the integrating the existing HDA jack kctl and soc jack
> kctl possible.
> 
> Signed-off-by: Jie Yang <yang.jie@intel.com>
> ---
>  include/sound/jack.h            |  4 +--
>  sound/core/jack.c               | 56 +++++++++++++++++++++++++----------------
>  sound/pci/hda/hda_jack.c        |  2 +-
>  sound/pci/oxygen/xonar_wm87x6.c |  2 +-
>  sound/soc/soc-jack.c            |  2 +-
>  5 files changed, 39 insertions(+), 27 deletions(-)
> 
> diff --git a/include/sound/jack.h b/include/sound/jack.h
> index 9781e75..34b6849 100644
> --- a/include/sound/jack.h
> +++ b/include/sound/jack.h
> @@ -93,7 +93,7 @@ struct snd_jack_kctl {
>  #ifdef CONFIG_SND_JACK
>  
>  int snd_jack_new(struct snd_card *card, const char *id, int type,
> -		 struct snd_jack **jack);
> +		 struct snd_jack **jack, bool phantom_jack, struct snd_jack_kctl **jjack_kctl);
>  int snd_jack_add_new_kctl(struct snd_jack *jack, const char * name, int mask);
>  void snd_jack_set_parent(struct snd_jack *jack, struct device *parent);
>  int snd_jack_set_key(struct snd_jack *jack, enum snd_jack_types type,
> @@ -103,7 +103,7 @@ void snd_jack_report(struct snd_jack *jack, int status);
>  
>  #else
>  static inline int snd_jack_new(struct snd_card *card, const char *id, int type,
> -			       struct snd_jack **jack)
> +			       struct snd_jack **jack, bool phantom_jack, struct snd_jack_kctl **jjack_kctl)
>  {
>  	return 0;
>  }
> diff --git a/sound/core/jack.c b/sound/core/jack.c
> index b13d0b1..edd55c8 100644
> --- a/sound/core/jack.c
> +++ b/sound/core/jack.c
> @@ -198,6 +198,10 @@ EXPORT_SYMBOL(snd_jack_add_new_kctl);
>   * @type:  a bitmask of enum snd_jack_type values that can be detected by
>   *         this jack
>   * @jjack: Used to provide the allocated jack object to the caller.
> + * @phantom_jack: for phantom jack, only create needed kctl, won't create
> + *         real jackdevice
> + * @jjack_kctl: create kctl if non-NULL pointer passed in, and provide it to
> + *         the caller. also add it to the non-phantom jack kctl list
>   *
>   * Creates a new jack object.
>   *
> @@ -205,7 +209,7 @@ EXPORT_SYMBOL(snd_jack_add_new_kctl);
>   * On success @jjack will be initialised.
>   */
>  int snd_jack_new(struct snd_card *card, const char *id, int type,
> -		 struct snd_jack **jjack)
> +		 struct snd_jack **jjack, bool phantom_jack, struct snd_jack_kctl **jjack_kctl)

The caller doesn't need to get struct snd_jack_kctl.  Instead, it's
better to get struct snd_kcontrol.  In that way, you can move the
definition of struct snd_jack_kctl locally into jack.c.  It's merely
an internal object only for jack after all.

>  {
>  	struct snd_jack *jack;
>  	int err;
> @@ -216,35 +220,43 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
>  		.dev_disconnect = snd_jack_dev_disconnect,
>  	};
>  
> -	jack = kzalloc(sizeof(struct snd_jack), GFP_KERNEL);
> -	if (jack == NULL)
> -		return -ENOMEM;
> +	if (jjack_kctl)
> +		*jjack_kctl = snd_jack_kctl_new(card, id, type);
>  
> -	jack->id = kstrdup(id, GFP_KERNEL);
> +	/* don't creat real jack device for phantom jack */
> +	if (!phantom_jack) {
> +		jack = kzalloc(sizeof(struct snd_jack), GFP_KERNEL);

We need to create a jack object even for a phantom jack.  It's the
place managing the list of kctls.  Otherwise we can't track these
kctls.

Just skip creating jack->input_dev for phantom jacks.


Takashi
Jie, Yang April 22, 2015, 2:06 a.m. UTC | #2
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Tuesday, April 21, 2015 5:57 PM
> To: Jie, Yang
> Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R
> Subject: Re: [PATCH v7 3/7] ALSA: jack: extend snd_jack_new to support
> phantom jack
> 
> At Tue, 21 Apr 2015 16:05:41 +0800,
> Jie Yang wrote:
> >
> > For phantom jack, we won't create real jack device, but jack kctl may
> > be needed.
> >
> > Here, we extend snd_jack_new() to support phantom jack creating:
> > pass in a bool param for [non-]phantom flag, and a non-Null param
> > (struct snd_jack_kctl **) to indicate that we need create kctl at this
> > jack creating stage.
> >
> > We can also add kctl to a jack after the jack is created.
> >
> > This make the integrating the existing HDA jack kctl and soc jack kctl
> > possible.
> >
> > Signed-off-by: Jie Yang <yang.jie@intel.com>
> > ---
> >  include/sound/jack.h            |  4 +--
> >  sound/core/jack.c               | 56 +++++++++++++++++++++++++--------------
> --
> >  sound/pci/hda/hda_jack.c        |  2 +-
> >  sound/pci/oxygen/xonar_wm87x6.c |  2 +-
> >  sound/soc/soc-jack.c            |  2 +-
> >  5 files changed, 39 insertions(+), 27 deletions(-)
> >
> > diff --git a/include/sound/jack.h b/include/sound/jack.h index
> > 9781e75..34b6849 100644
> > --- a/include/sound/jack.h
> > +++ b/include/sound/jack.h
> > @@ -93,7 +93,7 @@ struct snd_jack_kctl {  #ifdef CONFIG_SND_JACK
> >
> >  int snd_jack_new(struct snd_card *card, const char *id, int type,
> > -		 struct snd_jack **jack);
> > +		 struct snd_jack **jack, bool phantom_jack, struct
> snd_jack_kctl
> > +**jjack_kctl);
> >  int snd_jack_add_new_kctl(struct snd_jack *jack, const char * name,
> > int mask);  void snd_jack_set_parent(struct snd_jack *jack, struct
> > device *parent);  int snd_jack_set_key(struct snd_jack *jack, enum
> > snd_jack_types type, @@ -103,7 +103,7 @@ void snd_jack_report(struct
> > snd_jack *jack, int status);
> >
> >  #else
> >  static inline int snd_jack_new(struct snd_card *card, const char *id, int
> type,
> > -			       struct snd_jack **jack)
> > +			       struct snd_jack **jack, bool phantom_jack, struct
> > +snd_jack_kctl **jjack_kctl)
> >  {
> >  	return 0;
> >  }
> > diff --git a/sound/core/jack.c b/sound/core/jack.c index
> > b13d0b1..edd55c8 100644
> > --- a/sound/core/jack.c
> > +++ b/sound/core/jack.c
> > @@ -198,6 +198,10 @@ EXPORT_SYMBOL(snd_jack_add_new_kctl);
> >   * @type:  a bitmask of enum snd_jack_type values that can be detected
> by
> >   *         this jack
> >   * @jjack: Used to provide the allocated jack object to the caller.
> > + * @phantom_jack: for phantom jack, only create needed kctl, won't
> create
> > + *         real jackdevice
> > + * @jjack_kctl: create kctl if non-NULL pointer passed in, and provide it to
> > + *         the caller. also add it to the non-phantom jack kctl list
> >   *
> >   * Creates a new jack object.
> >   *
> > @@ -205,7 +209,7 @@ EXPORT_SYMBOL(snd_jack_add_new_kctl);
> >   * On success @jjack will be initialised.
> >   */
> >  int snd_jack_new(struct snd_card *card, const char *id, int type,
> > -		 struct snd_jack **jjack)
> > +		 struct snd_jack **jjack, bool phantom_jack, struct
> snd_jack_kctl
> > +**jjack_kctl)
> 
> The caller doesn't need to get struct snd_jack_kctl.  Instead, it's better to get
> struct snd_kcontrol.  In that way, you can move the definition of struct
> snd_jack_kctl locally into jack.c.  It's merely an internal object only for jack
> after all.
 
That's good. Will change it.

> 
> >  {
> >  	struct snd_jack *jack;
> >  	int err;
> > @@ -216,35 +220,43 @@ int snd_jack_new(struct snd_card *card, const
> char *id, int type,
> >  		.dev_disconnect = snd_jack_dev_disconnect,
> >  	};
> >
> > -	jack = kzalloc(sizeof(struct snd_jack), GFP_KERNEL);
> > -	if (jack == NULL)
> > -		return -ENOMEM;
> > +	if (jjack_kctl)
> > +		*jjack_kctl = snd_jack_kctl_new(card, id, type);
> >
> > -	jack->id = kstrdup(id, GFP_KERNEL);
> > +	/* don't creat real jack device for phantom jack */
> > +	if (!phantom_jack) {
> > +		jack = kzalloc(sizeof(struct snd_jack), GFP_KERNEL);
> 
> We need to create a jack object even for a phantom jack.  It's the place
> managing the list of kctls.  Otherwise we can't track these kctls.
> 
> Just skip creating jack->input_dev for phantom jacks.
 
Sounds good, if the extra jack devices have no side effect.

> 
> 
> Takashi
Jie, Yang April 22, 2015, 2:51 a.m. UTC | #3
> -----Original Message-----
> From: Jie, Yang
> Sent: Wednesday, April 22, 2015 10:07 AM
> To: 'Takashi Iwai'
> Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R
> Subject: RE: [PATCH v7 3/7] ALSA: jack: extend snd_jack_new to support
> phantom jack
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Tuesday, April 21, 2015 5:57 PM
> > To: Jie, Yang
> > Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R
> > Subject: Re: [PATCH v7 3/7] ALSA: jack: extend snd_jack_new to support
> > phantom jack
> >
> > At Tue, 21 Apr 2015 16:05:41 +0800,
> > Jie Yang wrote:
> > >
> > > For phantom jack, we won't create real jack device, but jack kctl
> > > may be needed.
> > >
> > > Here, we extend snd_jack_new() to support phantom jack creating:
> > > pass in a bool param for [non-]phantom flag, and a non-Null param
> > > (struct snd_jack_kctl **) to indicate that we need create kctl at
> > > this jack creating stage.
> > >
> > > We can also add kctl to a jack after the jack is created.
> > >
> > > This make the integrating the existing HDA jack kctl and soc jack
> > > kctl possible.
> > >
> > > Signed-off-by: Jie Yang <yang.jie@intel.com>
> > > ---
> > >  include/sound/jack.h            |  4 +--
> > >  sound/core/jack.c               | 56 +++++++++++++++++++++++++------------
> --
> > --
> > >  sound/pci/hda/hda_jack.c        |  2 +-
> > >  sound/pci/oxygen/xonar_wm87x6.c |  2 +-
> > >  sound/soc/soc-jack.c            |  2 +-
> > >  5 files changed, 39 insertions(+), 27 deletions(-)
> > >
> > > diff --git a/include/sound/jack.h b/include/sound/jack.h index
> > > 9781e75..34b6849 100644
> > > --- a/include/sound/jack.h
> > > +++ b/include/sound/jack.h
> > > @@ -93,7 +93,7 @@ struct snd_jack_kctl {  #ifdef CONFIG_SND_JACK
> > >
> > >  int snd_jack_new(struct snd_card *card, const char *id, int type,
> > > -		 struct snd_jack **jack);
> > > +		 struct snd_jack **jack, bool phantom_jack, struct
> > snd_jack_kctl
> > > +**jjack_kctl);
> > >  int snd_jack_add_new_kctl(struct snd_jack *jack, const char * name,
> > > int mask);  void snd_jack_set_parent(struct snd_jack *jack, struct
> > > device *parent);  int snd_jack_set_key(struct snd_jack *jack, enum
> > > snd_jack_types type, @@ -103,7 +103,7 @@ void snd_jack_report(struct
> > > snd_jack *jack, int status);
> > >
> > >  #else
> > >  static inline int snd_jack_new(struct snd_card *card, const char
> > > *id, int
> > type,
> > > -			       struct snd_jack **jack)
> > > +			       struct snd_jack **jack, bool phantom_jack, struct
> > > +snd_jack_kctl **jjack_kctl)
> > >  {
> > >  	return 0;
> > >  }
> > > diff --git a/sound/core/jack.c b/sound/core/jack.c index
> > > b13d0b1..edd55c8 100644
> > > --- a/sound/core/jack.c
> > > +++ b/sound/core/jack.c
> > > @@ -198,6 +198,10 @@ EXPORT_SYMBOL(snd_jack_add_new_kctl);
> > >   * @type:  a bitmask of enum snd_jack_type values that can be
> > > detected
> > by
> > >   *         this jack
> > >   * @jjack: Used to provide the allocated jack object to the caller.
> > > + * @phantom_jack: for phantom jack, only create needed kctl, won't
> > create
> > > + *         real jackdevice
> > > + * @jjack_kctl: create kctl if non-NULL pointer passed in, and provide it
> to
> > > + *         the caller. also add it to the non-phantom jack kctl list
> > >   *
> > >   * Creates a new jack object.
> > >   *
> > > @@ -205,7 +209,7 @@ EXPORT_SYMBOL(snd_jack_add_new_kctl);
> > >   * On success @jjack will be initialised.
> > >   */
> > >  int snd_jack_new(struct snd_card *card, const char *id, int type,
> > > -		 struct snd_jack **jjack)
> > > +		 struct snd_jack **jjack, bool phantom_jack, struct
> > snd_jack_kctl
> > > +**jjack_kctl)
> >
> > The caller doesn't need to get struct snd_jack_kctl.  Instead, it's
> > better to get struct snd_kcontrol.  In that way, you can move the
> > definition of struct snd_jack_kctl locally into jack.c.  It's merely
> > an internal object only for jack after all.
> 
> That's good. Will change it.
 
We can change it more, caller even don't need struct snd_kcontrol,
I am think of using a bool initial_kctl to indicate if we need create
kcontrol with jack id at this jack creating stage.

Then Jack will handle kcontrol totally, caller don't need care them
at all.

> 
> >
> > >  {
> > >  	struct snd_jack *jack;
> > >  	int err;
> > > @@ -216,35 +220,43 @@ int snd_jack_new(struct snd_card *card, const
> > char *id, int type,
> > >  		.dev_disconnect = snd_jack_dev_disconnect,
> > >  	};
> > >
> > > -	jack = kzalloc(sizeof(struct snd_jack), GFP_KERNEL);
> > > -	if (jack == NULL)
> > > -		return -ENOMEM;
> > > +	if (jjack_kctl)
> > > +		*jjack_kctl = snd_jack_kctl_new(card, id, type);
> > >
> > > -	jack->id = kstrdup(id, GFP_KERNEL);
> > > +	/* don't creat real jack device for phantom jack */
> > > +	if (!phantom_jack) {
> > > +		jack = kzalloc(sizeof(struct snd_jack), GFP_KERNEL);
> >
> > We need to create a jack object even for a phantom jack.  It's the
> > place managing the list of kctls.  Otherwise we can't track these kctls.
> >
> > Just skip creating jack->input_dev for phantom jacks.
> 
> Sounds good, if the extra jack devices have no side effect.
> 
> >
> >
> > Takashi
diff mbox

Patch

diff --git a/include/sound/jack.h b/include/sound/jack.h
index 9781e75..34b6849 100644
--- a/include/sound/jack.h
+++ b/include/sound/jack.h
@@ -93,7 +93,7 @@  struct snd_jack_kctl {
 #ifdef CONFIG_SND_JACK
 
 int snd_jack_new(struct snd_card *card, const char *id, int type,
-		 struct snd_jack **jack);
+		 struct snd_jack **jack, bool phantom_jack, struct snd_jack_kctl **jjack_kctl);
 int snd_jack_add_new_kctl(struct snd_jack *jack, const char * name, int mask);
 void snd_jack_set_parent(struct snd_jack *jack, struct device *parent);
 int snd_jack_set_key(struct snd_jack *jack, enum snd_jack_types type,
@@ -103,7 +103,7 @@  void snd_jack_report(struct snd_jack *jack, int status);
 
 #else
 static inline int snd_jack_new(struct snd_card *card, const char *id, int type,
-			       struct snd_jack **jack)
+			       struct snd_jack **jack, bool phantom_jack, struct snd_jack_kctl **jjack_kctl)
 {
 	return 0;
 }
diff --git a/sound/core/jack.c b/sound/core/jack.c
index b13d0b1..edd55c8 100644
--- a/sound/core/jack.c
+++ b/sound/core/jack.c
@@ -198,6 +198,10 @@  EXPORT_SYMBOL(snd_jack_add_new_kctl);
  * @type:  a bitmask of enum snd_jack_type values that can be detected by
  *         this jack
  * @jjack: Used to provide the allocated jack object to the caller.
+ * @phantom_jack: for phantom jack, only create needed kctl, won't create
+ *         real jackdevice
+ * @jjack_kctl: create kctl if non-NULL pointer passed in, and provide it to
+ *         the caller. also add it to the non-phantom jack kctl list
  *
  * Creates a new jack object.
  *
@@ -205,7 +209,7 @@  EXPORT_SYMBOL(snd_jack_add_new_kctl);
  * On success @jjack will be initialised.
  */
 int snd_jack_new(struct snd_card *card, const char *id, int type,
-		 struct snd_jack **jjack)
+		 struct snd_jack **jjack, bool phantom_jack, struct snd_jack_kctl **jjack_kctl)
 {
 	struct snd_jack *jack;
 	int err;
@@ -216,35 +220,43 @@  int snd_jack_new(struct snd_card *card, const char *id, int type,
 		.dev_disconnect = snd_jack_dev_disconnect,
 	};
 
-	jack = kzalloc(sizeof(struct snd_jack), GFP_KERNEL);
-	if (jack == NULL)
-		return -ENOMEM;
+	if (jjack_kctl)
+		*jjack_kctl = snd_jack_kctl_new(card, id, type);
 
-	jack->id = kstrdup(id, GFP_KERNEL);
+	/* don't creat real jack device for phantom jack */
+	if (!phantom_jack) {
+		jack = kzalloc(sizeof(struct snd_jack), GFP_KERNEL);
+		if (jack == NULL)
+			return -ENOMEM;
 
-	jack->input_dev = input_allocate_device();
-	if (jack->input_dev == NULL) {
-		err = -ENOMEM;
-		goto fail_input;
-	}
+		jack->id = kstrdup(id, GFP_KERNEL);
+
+		jack->input_dev = input_allocate_device();
+		if (jack->input_dev == NULL) {
+			err = -ENOMEM;
+			goto fail_input;
+		}
 
-	jack->input_dev->phys = "ALSA";
+		jack->input_dev->phys = "ALSA";
 
-	jack->type = type;
+		jack->type = type;
 
-	for (i = 0; i < SND_JACK_SWITCH_TYPES; i++)
-		if (type & (1 << i))
-			input_set_capability(jack->input_dev, EV_SW,
-					     jack_switch_types[i]);
+		for (i = 0; i < SND_JACK_SWITCH_TYPES; i++)
+			if (type & (1 << i))
+				input_set_capability(jack->input_dev, EV_SW,
+						     jack_switch_types[i]);
 
-	err = snd_device_new(card, SNDRV_DEV_JACK, jack, &ops);
-	if (err < 0)
-		goto fail_input;
+		err = snd_device_new(card, SNDRV_DEV_JACK, jack, &ops);
+		if (err < 0)
+			goto fail_input;
 
-	jack->card = card;
-	INIT_LIST_HEAD(&jack->kctl_list);
+		jack->card = card;
+		INIT_LIST_HEAD(&jack->kctl_list);
 
-	*jjack = jack;
+		if (jjack_kctl && *jjack_kctl)
+			snd_jack_kctl_add(jack, *jjack_kctl);
+		*jjack = jack;
+	}
 
 	return 0;
 
diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
index a046e2f..f45c488 100644
--- a/sound/pci/hda/hda_jack.c
+++ b/sound/pci/hda/hda_jack.c
@@ -417,7 +417,7 @@  static int __snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid,
 	if (!phantom_jack) {
 		jack->type = get_input_jack_type(codec, nid);
 		err = snd_jack_new(codec->bus->card, name, jack->type,
-				   &jack->jack);
+				   &jack->jack, false, NULL);
 		if (err < 0)
 			return err;
 		jack->jack->private_data = jack;
diff --git a/sound/pci/oxygen/xonar_wm87x6.c b/sound/pci/oxygen/xonar_wm87x6.c
index 6ce6860..a37450b 100644
--- a/sound/pci/oxygen/xonar_wm87x6.c
+++ b/sound/pci/oxygen/xonar_wm87x6.c
@@ -286,7 +286,7 @@  static void xonar_ds_init(struct oxygen *chip)
 	xonar_enable_output(chip);
 
 	snd_jack_new(chip->card, "Headphone",
-		     SND_JACK_HEADPHONE, &data->hp_jack);
+		     SND_JACK_HEADPHONE, &data->hp_jack, false, NULL);
 	xonar_ds_handle_hp_jack(chip);
 
 	snd_component_add(chip->card, "WM8776");
diff --git a/sound/soc/soc-jack.c b/sound/soc/soc-jack.c
index 9f60c25..836368b 100644
--- a/sound/soc/soc-jack.c
+++ b/sound/soc/soc-jack.c
@@ -48,7 +48,7 @@  int snd_soc_card_jack_new(struct snd_soc_card *card, const char *id, int type,
 	INIT_LIST_HEAD(&jack->jack_zones);
 	BLOCKING_INIT_NOTIFIER_HEAD(&jack->notifier);
 
-	ret = snd_jack_new(card->snd_card, id, type, &jack->jack);
+	ret = snd_jack_new(card->snd_card, id, type, &jack->jack, false, NULL);
 	if (ret)
 		return ret;