[PATCH 0/3] Add support simple-card parse jack detection via external codec

[PATCH 0/3] Add support simple-card parse jack detection via external codec

[Xing Zheng]

Hi,
    In most cases, many codecs already supports jack detection,
previouslly, we need to create a customized machine driver every time.

    Hence, the simple-card need to support use them dynamically via
parse dts file for better flexibility.


Xing Zheng (3):
  ASoC: jack: Add a jack detect callback via codec
  ASoC: simple-card: Add support jack detection via codec
  ASoC: da7219: Add detect_jack callback in the snd_soc_codec_driver

 .../devicetree/bindings/sound/simple-card.txt      |   17 +++
 include/sound/soc.h                                |    6 +
 sound/soc/codecs/da7219.c                          |    2 +
 sound/soc/generic/simple-card.c                    |  126 +++++++++++++++++++-
 sound/soc/soc-jack.c                               |    8 ++
 5 files changed, 156 insertions(+), 3 deletions(-)

-- 
1.7.9.5

[PATCH 1/3] ASoC: jack: Add a jack detect callback via codec

[Xing Zheng]

This patch add a callback when a codec have the jack detect feature.

Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
---

 include/sound/soc.h  |    6 ++++++
 sound/soc/soc-jack.c |    8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 02b4a21..ff105a4 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -547,6 +547,10 @@ static inline void snd_soc_jack_free_gpios(struct snd_soc_jack *jack, int count,
 }
 #endif
 
+/* init jack detect via codec */
+void snd_soc_jack_codec_detect(struct snd_soc_codec *codec,
+				struct snd_soc_jack *jack);
+
 /* codec register bit access */
 int snd_soc_update_bits(struct snd_soc_codec *codec, unsigned int reg,
 				unsigned int mask, unsigned int value);
@@ -920,6 +924,8 @@ struct snd_soc_codec_driver {
 			     enum snd_soc_dapm_type, int);
 
 	bool ignore_pmdown_time;  /* Doesn't benefit from pmdown delay */
+
+	void (*detect_jack)(struct snd_soc_codec *codec, struct snd_soc_jack *jack);
 };
 
 /* SoC platform interface */
diff --git a/sound/soc/soc-jack.c b/sound/soc/soc-jack.c
index fbaa1bb..e292fb4 100644
--- a/sound/soc/soc-jack.c
+++ b/sound/soc/soc-jack.c
@@ -436,3 +436,11 @@ void snd_soc_jack_free_gpios(struct snd_soc_jack *jack, int count,
 }
 EXPORT_SYMBOL_GPL(snd_soc_jack_free_gpios);
 #endif	/* CONFIG_GPIOLIB */
+
+void snd_soc_jack_codec_detect(struct snd_soc_codec *codec,
+				struct snd_soc_jack *jack)
+{
+	if (codec && codec->driver && codec->driver->detect_jack)
+		codec->driver->detect_jack(codec, jack);
+}
+EXPORT_SYMBOL_GPL(snd_soc_jack_codec_detect);
-- 
1.7.9.5

[PATCH 2/3] ASoC: simple-card: Add support jack detection via codec

[Xing Zheng]

In most cases, many codecs already supports jack detection, previouslly,
we need to create a customized machine driver every time.

Hence, the simple-card need to support use them dynamically via parse dts
file for better flexibility.

Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
---

 .../devicetree/bindings/sound/simple-card.txt      |   17 +++
 sound/soc/generic/simple-card.c                    |  126 +++++++++++++++++++-
 2 files changed, 140 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
index cf3979e..d25c8eb 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.txt
+++ b/Documentation/devicetree/bindings/sound/simple-card.txt
@@ -22,6 +22,23 @@ Optional properties:
 					  headphones are attached.
 - simple-audio-card,mic-det-gpio	: Reference to GPIO that signals when
 					  a microphone is attached.
+- simple-audio-card,codec-jack		: A list of the codec supports jack detection.
+					  The jack types which are supported refer to include/sound/jack.h.
+					  All of the jack types:
+						"JACK_HEADPHONE",
+						"JACK_MICROPHONE",
+						"JACK_HEADSET",
+						"JACK_LINEOUT",
+						"JACK_MECHANICAL",
+						"JACK_VIDEOOUT",
+						"JACK_AVOUT",
+						"JACK_LINEIN",
+						"JACK_BTN_0",
+						"JACK_BTN_1",
+						"JACK_BTN_2",
+						"JACK_BTN_3",
+						"JACK_BTN_4",
+						"JACK_BTN_5".
 
 Optional subnodes:
 
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 2389ab4..bc61022 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -33,12 +33,35 @@ struct simple_card_data {
 	int gpio_hp_det_invert;
 	int gpio_mic_det;
 	int gpio_mic_det_invert;
+	struct simple_codecs_jack {
+		int jack_det;
+		int jack_types;
+		struct snd_soc_jack codec_jack;
+	} *codec_det_jack;
 	struct snd_soc_dai_link dai_link[];	/* dynamically allocated */
 };
 
 #define simple_priv_to_dev(priv) ((priv)->snd_card.dev)
 #define simple_priv_to_link(priv, i) ((priv)->snd_card.dai_link + i)
 #define simple_priv_to_props(priv, i) ((priv)->dai_props + i)
+#define simple_priv_to_codecdetjack(priv, i) ((priv)->codec_det_jack + i)
+
+static const char * const jack_types_list[] = {
+	"JACK_HEADPHONE",
+	"JACK_MICROPHONE",
+	"JACK_HEADSET",
+	"JACK_LINEOUT",
+	"JACK_MECHANICAL",
+	"JACK_VIDEOOUT",
+	"JACK_AVOUT",
+	"JACK_LINEIN",
+	"JACK_BTN_0",
+	"JACK_BTN_1",
+	"JACK_BTN_2",
+	"JACK_BTN_3",
+	"JACK_BTN_4",
+	"JACK_BTN_5",
+};
 
 static int asoc_simple_card_startup(struct snd_pcm_substream *substream)
 {
@@ -136,6 +159,8 @@ static struct snd_soc_jack_gpio simple_card_mic_jack_gpio = {
 	.debounce_time = 150,
 };
 
+static struct snd_soc_jack simple_card_codecs_jack;
+
 static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
 				       struct asoc_simple_dai *set)
 {
@@ -173,9 +198,12 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd)
 	struct snd_soc_dai *codec = rtd->codec_dai;
 	struct snd_soc_dai *cpu = rtd->cpu_dai;
 	struct simple_dai_props *dai_props;
+	struct simple_codecs_jack *codec_det_jack;
 	int ret;
 
 	dai_props = &priv->dai_props[rtd->num];
+	codec_det_jack = &priv->codec_det_jack[rtd->num];
+
 	ret = __asoc_simple_card_dai_init(codec, &dai_props->codec_dai);
 	if (ret < 0)
 		return ret;
@@ -208,6 +236,80 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd)
 		snd_soc_jack_add_gpios(&simple_card_mic_jack, 1,
 				       &simple_card_mic_jack_gpio);
 	}
+
+	if (codec_det_jack->jack_det) {
+		ret = snd_soc_card_jack_new(rtd->card, "Simple-card Codec Jack",
+					    codec_det_jack->jack_types,
+					    &simple_card_codecs_jack, NULL, 0);
+		if (ret) {
+			dev_err(rtd->card->dev, "New Simple-card Codec Jack failed! (%d)\n", ret);
+			return ret;
+		}
+
+		snd_soc_jack_codec_detect(rtd->codec, &simple_card_codecs_jack);
+	}
+
+	return 0;
+}
+
+static int asoc_simple_card_parse_jack(struct device_node *np, int *jack_types)
+{
+	const char *propname = "simple-audio-card,codec-jack";
+	const char *jack_name;
+	int num_jacks, i, j, ret;
+
+	/* init with invalid, don't need to de-init if get count strings failed. */
+	*jack_types = -1;
+
+	num_jacks = of_property_count_strings(np, propname);
+	if (num_jacks < 0) {
+		pr_err("simple-card: Property '%s' number is invalid\n", propname);
+		return -EINVAL;
+	}
+
+	/* init and clean value */
+	*jack_types = 0;
+
+	for (i = 0; i < num_jacks; i++) {
+		ret = of_property_read_string_index(np, propname, i, &jack_name);
+		if (ret) {
+			pr_err("simple-card: Property '%s' index %d read error: %d\n",
+				propname, i, ret);
+			return -EINVAL;
+		}
+
+		for (j = 0; j < ARRAY_SIZE(jack_types_list); j++) {
+			if (!strcmp(jack_name, "JACK_HEADPHONE"))
+				*jack_types |= SND_JACK_HEADPHONE;
+			else if (!strcmp(jack_name, "JACK_MICROPHONE"))
+				*jack_types |= SND_JACK_MICROPHONE;
+			else if (!strcmp(jack_name, "JACK_HEADSET"))
+				*jack_types |= SND_JACK_HEADSET;
+			else if (!strcmp(jack_name, "JACK_LINEOUT"))
+				*jack_types |= SND_JACK_LINEOUT;
+			else if (!strcmp(jack_name, "JACK_MECHANICAL"))
+				*jack_types |= SND_JACK_MECHANICAL;
+			else if (!strcmp(jack_name, "JACK_VIDEOOUT"))
+				*jack_types |= SND_JACK_VIDEOOUT;
+			else if (!strcmp(jack_name, "JACK_AVOUT"))
+				*jack_types |= SND_JACK_AVOUT;
+			else if (!strcmp(jack_name, "JACK_LINEIN"))
+				*jack_types |= SND_JACK_LINEIN;
+			else if (!strcmp(jack_name, "JACK_BTN_0"))
+				*jack_types |= SND_JACK_BTN_0;
+			else if (!strcmp(jack_name, "JACK_BTN_1"))
+				*jack_types |= SND_JACK_BTN_1;
+			else if (!strcmp(jack_name, "JACK_BTN_2"))
+				*jack_types |= SND_JACK_BTN_2;
+			else if (!strcmp(jack_name, "JACK_BTN_3"))
+				*jack_types |= SND_JACK_BTN_3;
+			else if (!strcmp(jack_name, "JACK_BTN_4"))
+				*jack_types |= SND_JACK_BTN_4;
+			else if (!strcmp(jack_name, "JACK_BTN_5"))
+				*jack_types |= SND_JACK_BTN_5;
+		}
+	}
+
 	return 0;
 }
 
@@ -216,7 +318,8 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
 			      struct asoc_simple_dai *dai,
 			      struct device_node **p_node,
 			      const char **name,
-			      int *args_count)
+			      int *args_count,
+			      struct simple_codecs_jack *codec_det_jack)
 {
 	struct of_phandle_args args;
 	struct clk *clk;
@@ -272,6 +375,13 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
 			dai->sysclk = clk_get_rate(clk);
 	}
 
+	if (codec_det_jack) {
+		codec_det_jack->jack_det = of_property_read_bool(np,
+						"simple-audio-card,codec-jack");
+		if (codec_det_jack->jack_det)
+			asoc_simple_card_parse_jack(np, &codec_det_jack->jack_types);
+	}
+
 	return 0;
 }
 
@@ -325,6 +435,7 @@ static int asoc_simple_card_dai_link_of(struct device_node *node,
 	struct device *dev = simple_priv_to_dev(priv);
 	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, idx);
 	struct simple_dai_props *dai_props = simple_priv_to_props(priv, idx);
+	struct simple_codecs_jack *codec_det_jack = simple_priv_to_codecdetjack(priv, idx);
 	struct device_node *cpu = NULL;
 	struct device_node *plat = NULL;
 	struct device_node *codec = NULL;
@@ -364,13 +475,15 @@ static int asoc_simple_card_dai_link_of(struct device_node *node,
 	ret = asoc_simple_card_sub_parse_of(cpu, &dai_props->cpu_dai,
 					    &dai_link->cpu_of_node,
 					    &dai_link->cpu_dai_name,
-					    &cpu_args);
+					    &cpu_args, NULL);
 	if (ret < 0)
 		goto dai_link_of_err;
 
 	ret = asoc_simple_card_sub_parse_of(codec, &dai_props->codec_dai,
 					    &dai_link->codec_of_node,
-					    &dai_link->codec_dai_name, NULL);
+					    &dai_link->codec_dai_name, NULL,
+					    codec_det_jack);
+
 	if (ret < 0)
 		goto dai_link_of_err;
 
@@ -565,6 +678,13 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
 	if (!priv->dai_props)
 		return -ENOMEM;
 
+	/* Get room for the other properties */
+	priv->codec_det_jack = devm_kzalloc(dev,
+			sizeof(*priv->codec_det_jack) * num_links,
+			GFP_KERNEL);
+	if (!priv->codec_det_jack)
+		return -ENOMEM;
+
 	if (np && of_device_is_available(np)) {
 
 		ret = asoc_simple_card_parse_of(np, priv);
-- 
1.7.9.5

[PATCH 3/3] ASoC: da7219: Add detect_jack callback in the snd_soc_codec_driver

[Xing Zheng]

This patch tell soc-jack that this codec supports jack detection.

Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
---

 sound/soc/codecs/da7219.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/soc/codecs/da7219.c b/sound/soc/codecs/da7219.c
index 81c0708..5a8ff1e 100644
--- a/sound/soc/codecs/da7219.c
+++ b/sound/soc/codecs/da7219.c
@@ -1767,6 +1767,8 @@ static struct snd_soc_codec_driver soc_codec_dev_da7219 = {
 	.num_dapm_widgets	= ARRAY_SIZE(da7219_dapm_widgets),
 	.dapm_routes		= da7219_audio_map,
 	.num_dapm_routes	= ARRAY_SIZE(da7219_audio_map),
+
+	.detect_jack		= da7219_aad_jack_det,
 };
 
 
-- 
1.7.9.5

Re: [PATCH 1/3] ASoC: jack: Add a jack detect callback via codec

[Mark Brown]

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

On Mon, Apr 18, 2016 at 04:17:30PM +0800, Xing Zheng wrote:
> This patch add a callback when a codec have the jack detect feature.

> +void snd_soc_jack_codec_detect(struct snd_soc_codec *codec,
> +				struct snd_soc_jack *jack)
> +{
> +	if (codec && codec->driver && codec->driver->detect_jack)
> +		codec->driver->detect_jack(codec, jack);
> +}
> +EXPORT_SYMBOL_GPL(snd_soc_jack_codec_detect);

I've no idea what this is supposed to do sorry, you need a much better
changelog and probably also a restructuring of the code.  This says it
is adding a callback but it doesn't do that, it adds an export of a
function that calls an operation in a driver with no explanation.

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

Re: [PATCH 1/3] ASoC: jack: Add a jack detect callback via codec

[Xing Zheng]

Hi Mark,

On 2016年04月18日 17:22, Mark Brown wrote:
> On Mon, Apr 18, 2016 at 04:17:30PM +0800, Xing Zheng wrote:
>> This patch add a callback when a codec have the jack detect feature.
>> +void snd_soc_jack_codec_detect(struct snd_soc_codec *codec,
>> +				struct snd_soc_jack *jack)
>> +{
>> +	if (codec&&  codec->driver&&  codec->driver->detect_jack)
>> +		codec->driver->detect_jack(codec, jack);
>> +}
>> +EXPORT_SYMBOL_GPL(snd_soc_jack_codec_detect);
> I've no idea what this is supposed to do sorry, you need a much better
> changelog and probably also a restructuring of the code.  This says it
> is adding a callback but it doesn't do that, it adds an export of a
> function that calls an operation in a driver with no explanation.
Yes, sorry to forgot to add description for this API.

The patch_1/3 add an export of a function that calls the _detect_jack_ 
function
when a codec supports jack detection and fill the _detect_jack_ function 
in the
struct snd_soc_codec_driver.

I will clean up the commit message.

Thanks.

-- 
- Xing Zheng

Re: [PATCH 1/3] ASoC: jack: Add a jack detect callback via codec

[Mark Brown]

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

On Mon, Apr 18, 2016 at 05:43:51PM +0800, Xing Zheng wrote:

> The patch_1/3 add an export of a function that calls the _detect_jack_
> function
> when a codec supports jack detection and fill the _detect_jack_ function in
> the
> struct snd_soc_codec_driver.

But why would anything ever call this function?  How is this supposed to
be used?

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

Re: [PATCH 1/3] ASoC: jack: Add a jack detect callback via codec

[Xing Zheng]

HI Mark,

On 2016年04月18日 17:53, Mark Brown wrote:
> On Mon, Apr 18, 2016 at 05:43:51PM +0800, Xing Zheng wrote:
>
>> The patch_1/3 add an export of a function that calls the _detect_jack_
>> function
>> when a codec supports jack detection and fill the _detect_jack_ function in
>> the
>> struct snd_soc_codec_driver.
> But why would anything ever call this function?  How is this supposed to
> be used?
In my opinion, this function is mainly to provide simple-card (patch_2/3).

This function will call and initialize jack detection if a codec 
supports it and fill the detect_jack function (patch_3/3 use it),
and, we don't need to create a customized machine driver to call the 
codec jack detection, the simple-card is able to
parse them.

Thanks.

-- 
- Xing Zheng

Re: [PATCH 1/3] ASoC: jack: Add a jack detect callback via codec

[Xing Zheng]

On 2016年04月18日 18:03, Xing Zheng wrote:
> HI Mark,
>
> On 2016年04月18日 17:53, Mark Brown wrote:
>> On Mon, Apr 18, 2016 at 05:43:51PM +0800, Xing Zheng wrote:
>>
>>> The patch_1/3 add an export of a function that calls the _detect_jack_
>>> function
>>> when a codec supports jack detection and fill the _detect_jack_ 
>>> function in
>>> the
>>> struct snd_soc_codec_driver.
>> But why would anything ever call this function?  How is this supposed to
>> be used?
> In my opinion, this function is mainly to provide simple-card 
> (patch_2/3).
>
> This function will call and initialize jack detection if a codec 
> supports it and fill the detect_jack function (patch_3/3 use it),
> and, we don't need to create a customized machine driver to call the 
> codec jack detection, the simple-card is able to
> parse them.
>
> Thanks.
>
The dts usage like this:

sound {
         ......
         simple-audio-card,dai-link@0 {
             format = "i2s";
             cpu {
                 sound-dai = <&i2s0>;
             };

             codec {
                 sound-dai = <&codec>;

                 simple-audio-card,codec-jack =
                     "JACK_HEADSET",
                     "JACK_BTN_0",
                     "JACK_BTN_1",
                     "JACK_BTN_2",
                     "JACK_BTN_3";
             };
         };
         ......
};

-- 
- Xing Zheng

Re: [PATCH 1/3] ASoC: jack: Add a jack detect callback via codec

[Mark Brown]

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

On Mon, Apr 18, 2016 at 06:20:00PM +0800, Xing Zheng wrote:
> On 2016年04月18日 18:03, Xing Zheng wrote:

> sound {
>         ......
>         simple-audio-card,dai-link@0 {
>             format = "i2s";
>             cpu {
>                 sound-dai = <&i2s0>;
>             };
> 
>             codec {
>                 sound-dai = <&codec>;
> 
>                 simple-audio-card,codec-jack =
>                     "JACK_HEADSET",
>                     "JACK_BTN_0",
>                     "JACK_BTN_1",
>                     "JACK_BTN_2",
>                     "JACK_BTN_3";
>             };
>         };

This seems like it's only half the job and worryingly close to Linux
internals.  In particular the fact that the binding is specific to
simple-card and the fact that it's being placed on the CODEC (rather
than a separate object that the CODEC references) so that we can't
combine multiple devices are both a concern.

Dylan Reid did have an earlier go at defining a binding for this:

  http://thread.gmane.org/gmane.linux.alsa.devel/138906

which went through a couple more iterations but the work on that seemed
to die off a bit.

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

Re: [PATCH 1/3] ASoC: jack: Add a jack detect callback via codec

[Dylan Reid]

On Mon, Apr 18, 2016 at 4:46 AM, Mark Brown <broonie@kernel.org> wrote:
> On Mon, Apr 18, 2016 at 06:20:00PM +0800, Xing Zheng wrote:
>> On 2016年04月18日 18:03, Xing Zheng wrote:
>
>> sound {
>>         ......
>>         simple-audio-card,dai-link@0 {
>>             format = "i2s";
>>             cpu {
>>                 sound-dai = <&i2s0>;
>>             };
>>
>>             codec {
>>                 sound-dai = <&codec>;
>>
>>                 simple-audio-card,codec-jack =
>>                     "JACK_HEADSET",
>>                     "JACK_BTN_0",
>>                     "JACK_BTN_1",
>>                     "JACK_BTN_2",
>>                     "JACK_BTN_3";
>>             };
>>         };
>
> This seems like it's only half the job and worryingly close to Linux
> internals.  In particular the fact that the binding is specific to
> simple-card and the fact that it's being placed on the CODEC (rather
> than a separate object that the CODEC references) so that we can't
> combine multiple devices are both a concern.
>
> Dylan Reid did have an earlier go at defining a binding for this:
>
>   http://thread.gmane.org/gmane.linux.alsa.devel/138906
>
> which went through a couple more iterations but the work on that seemed
> to die off a bit.

Thanks for forwarding this.  I did intend to get back to that.  But,
almost a year later, obviously I haven't.

Xing if you want to pick this up it would be great.  Lars had some
good points on that original discussion, which I think we can
integrate into a single solution.

Re: [PATCH 2/3] ASoC: simple-card: Add support jack detection via codec

[Rob Herring]

On Mon, Apr 18, 2016 at 04:17:31PM +0800, Xing Zheng wrote:
> In most cases, many codecs already supports jack detection, previouslly,
> we need to create a customized machine driver every time.
> 
> Hence, the simple-card need to support use them dynamically via parse dts
> file for better flexibility.
> 
> Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
> ---
> 
>  .../devicetree/bindings/sound/simple-card.txt      |   17 +++
>  sound/soc/generic/simple-card.c                    |  126 +++++++++++++++++++-
>  2 files changed, 140 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
> index cf3979e..d25c8eb 100644
> --- a/Documentation/devicetree/bindings/sound/simple-card.txt
> +++ b/Documentation/devicetree/bindings/sound/simple-card.txt
> @@ -22,6 +22,23 @@ Optional properties:
>  					  headphones are attached.
>  - simple-audio-card,mic-det-gpio	: Reference to GPIO that signals when
>  					  a microphone is attached.
> +- simple-audio-card,codec-jack		: A list of the codec supports jack detection.
> +					  The jack types which are supported refer to include/sound/jack.h.
> +					  All of the jack types:
> +						"JACK_HEADPHONE",
> +						"JACK_MICROPHONE",
> +						"JACK_HEADSET",
> +						"JACK_LINEOUT",
> +						"JACK_MECHANICAL",
> +						"JACK_VIDEOOUT",
> +						"JACK_AVOUT",
> +						"JACK_LINEIN",
> +						"JACK_BTN_0",
> +						"JACK_BTN_1",
> +						"JACK_BTN_2",
> +						"JACK_BTN_3",
> +						"JACK_BTN_4",
> +						"JACK_BTN_5".

simple-card is not so simple...

I think this information be part of the codec node if the codec 
provides the functionality. The binding should not be different whether 
a codec is part of simple-card or not. Also, I would make all these just 
boolean props. 

Rob

Re: [PATCH 2/3] ASoC: simple-card: Add support jack detection via codec

[Xing Zheng]

On 2016年04月21日 22:19, Rob Herring wrote:
> On Mon, Apr 18, 2016 at 04:17:31PM +0800, Xing Zheng wrote:
>> In most cases, many codecs already supports jack detection, previouslly,
>> we need to create a customized machine driver every time.
>>
>> Hence, the simple-card need to support use them dynamically via parse dts
>> file for better flexibility.
>>
>> Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
>> ---
>>
>>   .../devicetree/bindings/sound/simple-card.txt      |   17 +++
>>   sound/soc/generic/simple-card.c                    |  126 +++++++++++++++++++-
>>   2 files changed, 140 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
>> index cf3979e..d25c8eb 100644
>> --- a/Documentation/devicetree/bindings/sound/simple-card.txt
>> +++ b/Documentation/devicetree/bindings/sound/simple-card.txt
>> @@ -22,6 +22,23 @@ Optional properties:
>>   					  headphones are attached.
>>   - simple-audio-card,mic-det-gpio	: Reference to GPIO that signals when
>>   					  a microphone is attached.
>> +- simple-audio-card,codec-jack		: A list of the codec supports jack detection.
>> +					  The jack types which are supported refer to include/sound/jack.h.
>> +					  All of the jack types:
>> +						"JACK_HEADPHONE",
>> +						"JACK_MICROPHONE",
>> +						"JACK_HEADSET",
>> +						"JACK_LINEOUT",
>> +						"JACK_MECHANICAL",
>> +						"JACK_VIDEOOUT",
>> +						"JACK_AVOUT",
>> +						"JACK_LINEIN",
>> +						"JACK_BTN_0",
>> +						"JACK_BTN_1",
>> +						"JACK_BTN_2",
>> +						"JACK_BTN_3",
>> +						"JACK_BTN_4",
>> +						"JACK_BTN_5".
> simple-card is not so simple...
>
> I think this information be part of the codec node if the codec
> provides the functionality. The binding should not be different whether
> a codec is part of simple-card or not. Also, I would make all these just
> boolean props.
Yes, these things seem like make simple-card becomes more complicated, 
and the generic jack detection method is still a problem currently.

Hi Mark, the other question here, how do we make sense to use the 
simple-card to call the "snd_soc_dai_set_pll" if the codec (like da7219) 
requires do this?

Thanks.

-- 
- Xing Zheng

Re: [PATCH 2/3] ASoC: simple-card: Add support jack detection via codec

[Mark Brown]

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

On Fri, Apr 22, 2016 at 09:17:46AM +0800, Xing Zheng wrote:

> Hi Mark, the other question here, how do we make sense to use the
> simple-card to call the "snd_soc_dai_set_pll" if the codec (like da7219)
> requires do this?

What we should do there is move the CODEC clocking to be represented
using the clock API and then there are common clock bindings that
already exist for configuring clocks.

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

Re: [alsa-devel] [PATCH 2/3] ASoC: simple-card: Add support jack detection via codec

[Jeremy McDermond]

> On Apr 22, 2016, at 2:22 AM, Mark Brown <broonie@kernel.org> wrote:
> 
> On Fri, Apr 22, 2016 at 09:17:46AM +0800, Xing Zheng wrote:
> 
>> Hi Mark, the other question here, how do we make sense to use the
>> simple-card to call the "snd_soc_dai_set_pll" if the codec (like da7219)
>> requires do this?
> 
> What we should do there is move the CODEC clocking to be represented
> using the clock API and then there are common clock bindings that
> already exist for configuring clocks.

I’m a little bit curious about this.  I’m working with the clocking
on the tlv320aic32x4 driver because I’m dealing with a platform that
will be clocking it from a dynamic clock controlled by the CCF.  That
presents some problems because the aic32x4 driver has a table of clock
parameters and those rely on the mclk rate being a precise value.
The mclk might not be able to provide those exact values, so the
aic32x4 driver needs to set its PLLs and dividers accordingly.

The hardware has a couple of clock muxes, a few dividers and the
PLL.  It seems to be able to be modeled by the current CCF
components fairly well.  I’m just trying to wrap my head around
where this all would fit in.  Should the codec driver just register
a bunch of clock components in its initialization somewhere?
I’m thinking that it can’t have its own CCF driver separate from
the codec driver because you won’t have proper access to the
registers over I2C or SPI.  I don’t find a lot of examples in the
codec drivers, and there aren’t very many other drivers in the tree
that seem to register clocks with CCF that I can find.

I’m happy to work through this with the aic32x4 driver.  I just
need some guidance on how to do it so it fits in well with
everything else surrounding it.

--
Jeremy McDermond (NH6Z)
Xenotropic Systems
mcdermj@xenotropic.com

Re: [alsa-devel] [PATCH 2/3] ASoC: simple-card: Add support jack detection via codec

[Jeremy McDermond]

> On Apr 22, 2016, at 2:22 AM, Mark Brown <broonie@kernel.org> wrote:
> 
> On Fri, Apr 22, 2016 at 09:17:46AM +0800, Xing Zheng wrote:
> 
>> Hi Mark, the other question here, how do we make sense to use the
>> simple-card to call the "snd_soc_dai_set_pll" if the codec (like da7219)
>> requires do this?
> 
> What we should do there is move the CODEC clocking to be represented
> using the clock API and then there are common clock bindings that
> already exist for configuring clocks.

I’m a little bit curious about this.  I’m working with the clocking
on the tlv320aic32x4 driver because I’m dealing with a platform that
will be clocking it from a dynamic clock controlled by the CCF.  That
presents some problems because the aic32x4 driver has a table of clock
parameters and those rely on the mclk rate being a precise value.
The mclk might not be able to provide those exact values, so the
aic32x4 driver needs to set its PLLs and dividers accordingly.

The hardware has a couple of clock muxes, a few dividers and the
PLL.  It seems to be able to be modeled by the current CCF
components fairly well.  I’m just trying to wrap my head around
where this all would fit in.  Should the codec driver just register
a bunch of clock components in its initialization somewhere?
I’m thinking that it can’t have its own CCF driver separate from
the codec driver because you won’t have proper access to the
registers over I2C or SPI.  I don’t find a lot of examples in the
codec drivers, and there aren’t very many other drivers in the tree
that seem to register clocks with CCF that I can find.

I’m happy to work through this with the aic32x4 driver.  I just
need some guidance on how to do it so it fits in well with
everything else surrounding it.

--
Jeremy McDermond (NH6Z)
Xenotropic Systems
mcdermj@xenotropic.com

Re: [alsa-devel] [PATCH 2/3] ASoC: simple-card: Add support jack detection via codec

[Mark Brown]

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

On Fri, Apr 22, 2016 at 09:37:39AM -0700, Jeremy McDermond wrote:

> components fairly well.  I’m just trying to wrap my head around
> where this all would fit in.  Should the codec driver just register
> a bunch of clock components in its initialization somewhere?
> I’m thinking that it can’t have its own CCF driver separate from
> the codec driver because you won’t have proper access to the
> registers over I2C or SPI.  I don’t find a lot of examples in the
> codec drivers, and there aren’t very many other drivers in the tree
> that seem to register clocks with CCF that I can find.

If the clock code is worth splitting off into a separate driver that's
what drivers/mfd is for.

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

Re: [alsa-devel] [PATCH 2/3] ASoC: simple-card: Add support jack detection via codec

[Jeremy McDermond]

> On Apr 25, 2016, at 5:55 AM, Mark Brown <broonie@kernel.org> wrote:
> 
> If the clock code is worth splitting off into a separate driver that's
> what drivers/mfd is for.

I have my doubts that it’s worth splitting off into a separate
driver.  There’s not a lot of use for it outside of the internals of
the tlv320aic3204.  There is a provision for putting the PLL output
on a GPIO pin of the chip, but I’m not sure that’s a justification
for breaking out the clock stuff into its own driver.  Nobody’s going
to put a tlv320aic3204 chip on their boards because of the clocking;
they’re going to put it there to be a codec.

--
Jeremy McDermond (NH6Z)
Xenotropic Systems
mcdermj@xenotropic.com