[RFC][PATCH v4 0/4] design a way to change audio Jack state by software

[RFC][PATCH v4 0/4] design a way to change audio Jack state by software

[Hui Wang]

the changes in the v4:
 - change the sound-core to sound and change the sound_core_debugfs_root
   to sound_debugfs_root in the 0001-xxx.patch
 - change kzalloc/kfree to kvzalloc/kvfree in the debugfs fops for
   0001-xxx.patch and 0003-xxx.patch
 - And if needed, I could squash 4 patches into 1 patch before merging.

the changes in the v3 (for easy to review, divide change into 4 patches):
 - address the comment about the snd_jack_report() and _snd_jack_report(),
   the v2 design is hard to understand and is hard to review, in the v3,
   separate the jack_report to snd_jack_report() and snd_jack_inject_report(),
   hw jack events call snd_jack_report() as before, if a jack contains multi
   jack_kctl and the jack_kctl's sw_inject is enabled, the status and the
   related input-dev's events will not be reproted. The injection events call
   snd_jack_inject_report(). This change is squashed to 0001-xxx.patch,  the
   rest part of 0001-xxx.patch is same as v2.

 - address the comment about folders'name in the 0002-xxx.patch, so far, drop
   the '/', ',', '=' and ' ' from the folders' name.

 - address the comment about adding more debugfs nodes in the 0003-xxx.patch,
   it adds kctl_id, mask_bits, status and type.

 - address the comment about save-n-restore jack's hw status in the
   0004-xxx.patch, adding a hw_status_cache and save the last reported jack
   hw event, once the sw_inject is disabled, will restore all jack_kctl's
   state under the same snd_jack with hw_status_cache.
[snip]


the changes in the V2:
 - using debugfs instead of sysfs
 - using jack_ctrl to create a folder instead of snd_jack, since ASoC drivers
   could create multi jack_ctrls within a snd_jack
 - create a folder for each jack_ctrl instead for all jack_ctrls
[ snip ]


Hui Wang (4):
  alsa: jack: implement software jack injection via debugfs
  alsa: jack: adjust jack_kctl debugfs folder's name
  alsa: jack: add more jack_kctl debugfs nodes
  alsa: jack: implement save-and-restore for jack's hw status

 include/sound/core.h |   2 +
 include/sound/jack.h |   1 +
 sound/core/init.c    |   7 +
 sound/core/jack.c    | 352 ++++++++++++++++++++++++++++++++++++++++++-
 sound/core/sound.c   |   8 +
 5 files changed, 366 insertions(+), 4 deletions(-)

-- 
2.25.1

[RFC][PATCH v4 1/4] alsa: jack: implement software jack injection via debugfs

[Hui Wang]

We want to perform remote audio auto test, need the audio jack to
change from plugout to plugin or vice versa by software ways.

Here the design is creating a sound root folder in the debugfs
dir, and each sound card will create a folder cardN under sound,
then the sound jack will create folders by jack_ctrl->ctrl->id.name,
and will create 2 file nodes jackin_inject and sw_inject_enable in
the folder, this is the layout of folder on a machine with 2 sound
cards:
$tree $debugfs_mount_dir/sound
sound/
├── card0
│   ├── HDMI!DP,pcm=10 Jack
│   │   ├── jackin_inject
│   │   └── sw_inject_enable
│   ├── HDMI!DP,pcm=11 Jack
│   │   ├── jackin_inject
│   │   └── sw_inject_enable
│   ├── HDMI!DP,pcm=3 Jack
│   │   ├── jackin_inject
│   │   └── sw_inject_enable
│   ├── HDMI!DP,pcm=7 Jack
│   │   ├── jackin_inject
│   │   └── sw_inject_enable
│   ├── HDMI!DP,pcm=8 Jack
│   │   ├── jackin_inject
│   │   └── sw_inject_enable
│   └── HDMI!DP,pcm=9 Jack
│       ├── jackin_inject
│       └── sw_inject_enable
└── card1
    ├── HDMI!DP,pcm=3 Jack
    │   ├── jackin_inject
    │   └── sw_inject_enable
    ├── HDMI!DP,pcm=4 Jack
    │   ├── jackin_inject
    │   └── sw_inject_enable
    ├── HDMI!DP,pcm=5 Jack
    │   ├── jackin_inject
    │   └── sw_inject_enable
    ├── Headphone Jack
    │   ├── jackin_inject
    │   └── sw_inject_enable
    ├── Headset Jack
    │   ├── jackin_inject
    │   └── sw_inject_enable
    └── Mic Jack
        ├── jackin_inject
        └── sw_inject_enable

Suppose users want to enable jack injection for Headphone, they need
to run $sudo sh -c 'echo 1 > 'Headphone Jack'/sw_inject_enable', then
users could change the Headphone Jack state through jackin_inject and
this Jack's state will not be changed by non-injection ways anymore
until users echo 0 to sw_inject_enable.

Users could run $sudo sh -c 'echo 1 > 'Headphone Jack'/jackin_inject'
to trigger the Headphone jack to plugin or echo 0 to trigger it to
plugout.

If users finish their test, they could run
$sudo sh -c 'echo 0 > 'Headphone Jack'/sw_inject_enable' to disable
injection and let non-injection ways control this Jack.

For the jack event report, the hw jack event will call
snd_jack_report(), it will avoid to report the events if a jack_kctl's
sw_inject is enabled, also the related input_dev's events will not be
reproted.

If users inject a event, it will call snd_jack_inject_report(), it
only reports the events for this jack_kctl and reports the related
input_dev's events.

Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 include/sound/core.h |   2 +
 sound/core/init.c    |   7 ++
 sound/core/jack.c    | 182 ++++++++++++++++++++++++++++++++++++++++++-
 sound/core/sound.c   |   8 ++
 4 files changed, 195 insertions(+), 4 deletions(-)

diff --git a/include/sound/core.h b/include/sound/core.h
index 0462c577d7a3..15175fd44971 100644
--- a/include/sound/core.h
+++ b/include/sound/core.h
@@ -122,6 +122,7 @@ struct snd_card {
 
 	size_t total_pcm_alloc_bytes;	/* total amount of allocated buffers */
 	struct mutex memory_mutex;	/* protection for the above */
+	struct dentry *debugfs_root;    /* debugfs root for card */
 
 #ifdef CONFIG_PM
 	unsigned int power_state;	/* power state */
@@ -180,6 +181,7 @@ static inline struct device *snd_card_get_device_link(struct snd_card *card)
 extern int snd_major;
 extern int snd_ecards_limit;
 extern struct class *sound_class;
+extern struct dentry *sound_debugfs_root;
 
 void snd_request_card(int card);
 
diff --git a/sound/core/init.c b/sound/core/init.c
index 75aec71c48a8..d5ed2580f30c 100644
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -13,6 +13,7 @@
 #include <linux/time.h>
 #include <linux/ctype.h>
 #include <linux/pm.h>
+#include <linux/debugfs.h>
 #include <linux/completion.h>
 
 #include <sound/core.h>
@@ -161,6 +162,7 @@ int snd_card_new(struct device *parent, int idx, const char *xid,
 {
 	struct snd_card *card;
 	int err;
+	char name[8];
 
 	if (snd_BUG_ON(!card_ret))
 		return -EINVAL;
@@ -244,6 +246,10 @@ int snd_card_new(struct device *parent, int idx, const char *xid,
 		dev_err(parent, "unable to create card info\n");
 		goto __error_ctl;
 	}
+
+	sprintf(name, "card%d", idx);
+	card->debugfs_root = debugfs_create_dir(name, sound_debugfs_root);
+
 	*card_ret = card;
 	return 0;
 
@@ -416,6 +422,7 @@ int snd_card_disconnect(struct snd_card *card)
 	/* notify all devices that we are disconnected */
 	snd_device_disconnect_all(card);
 
+	debugfs_remove(card->debugfs_root);
 	snd_info_card_disconnect(card);
 	if (card->registered) {
 		device_del(&card->card_dev);
diff --git a/sound/core/jack.c b/sound/core/jack.c
index 503c8af79d55..0092cb6b5b79 100644
--- a/sound/core/jack.c
+++ b/sound/core/jack.c
@@ -8,6 +8,9 @@
 #include <linux/input.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/ctype.h>
+#include <linux/mm.h>
+#include <linux/debugfs.h>
 #include <sound/jack.h>
 #include <sound/core.h>
 #include <sound/control.h>
@@ -16,6 +19,9 @@ struct snd_jack_kctl {
 	struct snd_kcontrol *kctl;
 	struct list_head list;  /* list of controls belong to the same jack */
 	unsigned int mask_bits; /* only masked status bits are reported via kctl */
+	struct snd_jack *jack;  /* pointer to struct snd_jack */
+	bool sw_inject_enable;  /* allow to inject plug event via debugfs */
+	struct dentry *jack_debugfs_root; /* jack_kctl debugfs root */
 };
 
 #ifdef CONFIG_SND_JACK_INPUT_DEV
@@ -109,12 +115,172 @@ static int snd_jack_dev_register(struct snd_device *device)
 }
 #endif /* CONFIG_SND_JACK_INPUT_DEV */
 
+#ifdef CONFIG_DEBUG_FS
+static void snd_jack_inject_report(struct snd_jack_kctl *jack_kctl, int status)
+{
+	struct snd_jack *jack;
+#ifdef CONFIG_SND_JACK_INPUT_DEV
+	int i;
+#endif
+	if (!jack_kctl)
+		return;
+
+	jack = jack_kctl->jack;
+
+	if (jack_kctl->sw_inject_enable)
+		snd_kctl_jack_report(jack->card, jack_kctl->kctl,
+				     status & jack_kctl->mask_bits);
+
+#ifdef CONFIG_SND_JACK_INPUT_DEV
+	if (!jack->input_dev)
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(jack->key); i++) {
+		int testbit = ((SND_JACK_BTN_0 >> i) & jack_kctl->mask_bits);
+
+		if (jack->type & testbit)
+			input_report_key(jack->input_dev, jack->key[i],
+					 status & testbit);
+	}
+
+	for (i = 0; i < ARRAY_SIZE(jack_switch_types); i++) {
+		int testbit = ((1 << i) & jack_kctl->mask_bits);
+
+		if (jack->type & testbit)
+			input_report_switch(jack->input_dev,
+					    jack_switch_types[i],
+					    status & testbit);
+	}
+
+	input_sync(jack->input_dev);
+#endif /* CONFIG_SND_JACK_INPUT_DEV */
+}
+
+static ssize_t sw_inject_enable_read(struct file *file,
+				     char __user *to, size_t count, loff_t *ppos)
+{
+	struct snd_jack_kctl *jack_kctl = file->private_data;
+	char *buf;
+	int len, ret;
+
+	buf = kvzalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	len = scnprintf(buf, PAGE_SIZE, "%s: %s\t\t%s: %i\n", "Jack", jack_kctl->kctl->id.name,
+			"Inject Enabled", jack_kctl->sw_inject_enable);
+	ret = simple_read_from_buffer(to, count, ppos, buf, len);
+
+	kvfree(buf);
+	return ret;
+}
+
+static ssize_t sw_inject_enable_write(struct file *file,
+				      const char __user *from, size_t count, loff_t *ppos)
+{
+	struct snd_jack_kctl *jack_kctl = file->private_data;
+	char *buf;
+	int ret, err;
+	unsigned long enable;
+
+	buf = kvzalloc(count, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = simple_write_to_buffer(buf, count, ppos, from, count);
+	err = kstrtoul(buf, 0, &enable);
+	if (err) {
+		ret = err;
+		goto exit;
+	}
+
+	jack_kctl->sw_inject_enable = !!enable;
+
+ exit:
+	kvfree(buf);
+	return ret;
+}
+
+static ssize_t jackin_inject_write(struct file *file,
+				   const char __user *from, size_t count, loff_t *ppos)
+{
+	struct snd_jack_kctl *jack_kctl = file->private_data;
+	char *buf;
+	int ret, err;
+	unsigned long enable;
+
+	if (!jack_kctl->sw_inject_enable)
+		return -EINVAL;
+
+	buf = kvzalloc(count, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = simple_write_to_buffer(buf, count, ppos, from, count);
+	err = kstrtoul(buf, 0, &enable);
+	if (err) {
+		ret = err;
+		goto exit;
+	}
+
+	snd_jack_inject_report(jack_kctl, !!enable ? jack_kctl->mask_bits : 0);
+
+ exit:
+	kvfree(buf);
+	return ret;
+}
+
+static const struct file_operations sw_inject_enable_fops = {
+	.open = simple_open,
+	.read = sw_inject_enable_read,
+	.write = sw_inject_enable_write,
+	.llseek = default_llseek,
+};
+
+static const struct file_operations jackin_inject_fops = {
+	.open = simple_open,
+	.write = jackin_inject_write,
+	.llseek = default_llseek,
+};
+
+static int snd_jack_debugfs_add_inject_node(struct snd_jack *jack,
+					    struct snd_jack_kctl *jack_kctl)
+{
+	char *tname;
+
+	/* the folder's name can't contains '/', need to replace it with '!'
+	 * as lib/kobject.c does
+	 */
+	tname = kstrdup(jack_kctl->kctl->id.name, GFP_KERNEL);
+	if (!tname)
+		return -ENOMEM;
+	strreplace(tname, '/', '!');
+	jack_kctl->jack_debugfs_root = debugfs_create_dir(tname, jack->card->debugfs_root);
+	kfree(tname);
+
+	debugfs_create_file("sw_inject_enable", 0644, jack_kctl->jack_debugfs_root, jack_kctl,
+			    &sw_inject_enable_fops);
+
+	debugfs_create_file("jackin_inject", 0200, jack_kctl->jack_debugfs_root, jack_kctl,
+			    &jackin_inject_fops);
+
+	return 0;
+}
+#else /* CONFIG_DEBUG_FS */
+static int snd_jack_debugfs_add_inject_node(struct snd_jack *jack,
+					    struct snd_jack_kctl *jack_kctl)
+{
+	return 0;
+}
+#endif /* CONFIG_DEBUG_FS */
+
 static void snd_jack_kctl_private_free(struct snd_kcontrol *kctl)
 {
 	struct snd_jack_kctl *jack_kctl;
 
 	jack_kctl = kctl->private_data;
 	if (jack_kctl) {
+		debugfs_remove(jack_kctl->jack_debugfs_root);
 		list_del(&jack_kctl->list);
 		kfree(jack_kctl);
 	}
@@ -122,7 +288,10 @@ static void snd_jack_kctl_private_free(struct snd_kcontrol *kctl)
 
 static void snd_jack_kctl_add(struct snd_jack *jack, struct snd_jack_kctl *jack_kctl)
 {
+	jack_kctl->jack = jack;
 	list_add_tail(&jack_kctl->list, &jack->kctl_list);
+	if (!strstr(jack_kctl->kctl->id.name, "Phantom"))
+		snd_jack_debugfs_add_inject_node(jack, jack_kctl);
 }
 
 static struct snd_jack_kctl * snd_jack_kctl_new(struct snd_card *card, const char *name, unsigned int mask)
@@ -340,6 +509,7 @@ EXPORT_SYMBOL(snd_jack_set_key);
 void snd_jack_report(struct snd_jack *jack, int status)
 {
 	struct snd_jack_kctl *jack_kctl;
+	unsigned int mask_bits = 0;
 #ifdef CONFIG_SND_JACK_INPUT_DEV
 	int i;
 #endif
@@ -348,15 +518,18 @@ void snd_jack_report(struct snd_jack *jack, int status)
 		return;
 
 	list_for_each_entry(jack_kctl, &jack->kctl_list, list)
-		snd_kctl_jack_report(jack->card, jack_kctl->kctl,
-					    status & jack_kctl->mask_bits);
+		if (jack_kctl->sw_inject_enable)
+			mask_bits |= jack_kctl->mask_bits;
+		else
+			snd_kctl_jack_report(jack->card, jack_kctl->kctl,
+					     status & jack_kctl->mask_bits);
 
 #ifdef CONFIG_SND_JACK_INPUT_DEV
 	if (!jack->input_dev)
 		return;
 
 	for (i = 0; i < ARRAY_SIZE(jack->key); i++) {
-		int testbit = SND_JACK_BTN_0 >> i;
+		int testbit = ((SND_JACK_BTN_0 >> i) & ~mask_bits);
 
 		if (jack->type & testbit)
 			input_report_key(jack->input_dev, jack->key[i],
@@ -364,7 +537,8 @@ void snd_jack_report(struct snd_jack *jack, int status)
 	}
 
 	for (i = 0; i < ARRAY_SIZE(jack_switch_types); i++) {
-		int testbit = 1 << i;
+		int testbit = ((1 << i) & ~mask_bits);
+
 		if (jack->type & testbit)
 			input_report_switch(jack->input_dev,
 					    jack_switch_types[i],
diff --git a/sound/core/sound.c b/sound/core/sound.c
index b75f78f2c4b8..8a80f0440455 100644
--- a/sound/core/sound.c
+++ b/sound/core/sound.c
@@ -9,6 +9,7 @@
 #include <linux/time.h>
 #include <linux/device.h>
 #include <linux/module.h>
+#include <linux/debugfs.h>
 #include <sound/core.h>
 #include <sound/minors.h>
 #include <sound/info.h>
@@ -39,6 +40,9 @@ MODULE_ALIAS_CHARDEV_MAJOR(CONFIG_SND_MAJOR);
 int snd_ecards_limit;
 EXPORT_SYMBOL(snd_ecards_limit);
 
+struct dentry *sound_debugfs_root;
+EXPORT_SYMBOL_GPL(sound_debugfs_root);
+
 static struct snd_minor *snd_minors[SNDRV_OS_MINORS];
 static DEFINE_MUTEX(sound_mutex);
 
@@ -395,6 +399,9 @@ static int __init alsa_sound_init(void)
 		unregister_chrdev(major, "alsa");
 		return -ENOMEM;
 	}
+
+	sound_debugfs_root = debugfs_create_dir("sound", NULL);
+
 #ifndef MODULE
 	pr_info("Advanced Linux Sound Architecture Driver Initialized.\n");
 #endif
@@ -403,6 +410,7 @@ static int __init alsa_sound_init(void)
 
 static void __exit alsa_sound_exit(void)
 {
+	debugfs_remove(sound_debugfs_root);
 	snd_info_done();
 	unregister_chrdev(major, "alsa");
 }
-- 
2.25.1

Re: [RFC][PATCH v4 1/4] alsa: jack: implement software jack injection via debugfs

[Takashi Iwai]

On Mon, 11 Jan 2021 14:05:54 +0100,
Hui Wang wrote:
> +static ssize_t sw_inject_enable_read(struct file *file,
> +				     char __user *to, size_t count, loff_t *ppos)
> +{
> +	struct snd_jack_kctl *jack_kctl = file->private_data;
> +	char *buf;
> +	int len, ret;
> +
> +	buf = kvzalloc(PAGE_SIZE, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	len = scnprintf(buf, PAGE_SIZE, "%s: %s\t\t%s: %i\n", "Jack", jack_kctl->kctl->id.name,
> +			"Inject Enabled", jack_kctl->sw_inject_enable);
> +	ret = simple_read_from_buffer(to, count, ppos, buf, len);

For this size of a string buffer, you don't need to allocate via
kvmalloc().  It can be on stack.


> +static ssize_t sw_inject_enable_write(struct file *file,
> +				      const char __user *from, size_t count, loff_t *ppos)
> +{
> +	struct snd_jack_kctl *jack_kctl = file->private_data;
> +	char *buf;
> +	int ret, err;
> +	unsigned long enable;
> +
> +	buf = kvzalloc(count, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	ret = simple_write_to_buffer(buf, count, ppos, from, count);
> +	err = kstrtoul(buf, 0, &enable);

Ditto, it's just a single number and cannot be that big.
(Also jackin_inject_write(), too.)


thanks,

Takashi

[RFC][PATCH v4 2/4] alsa: jack: adjust jack_kctl debugfs folder's name

[Hui Wang]

We used jack_kctl->kctl.id as the folder's name, but there are some
characters which are not suitable for foler's name, for example, a
HDMI/DP audio jack id contains '/', ',', '=' and ' ', this patch will
remove them from folder's name.

Before applying patch, the folders look like:
'HDMI!DP,pcm=3 Jack'  'Headphone Jack'  'Mic Jack'

After applying the patch, the folders look like:
HDMIDPpcm3Jack  HeadphoneJack  MicJack

Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 sound/core/jack.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/sound/core/jack.c b/sound/core/jack.c
index 0092cb6b5b79..e1d1b26f3a5e 100644
--- a/sound/core/jack.c
+++ b/sound/core/jack.c
@@ -243,18 +243,36 @@ static const struct file_operations jackin_inject_fops = {
 	.llseek = default_llseek,
 };
 
+/* The substrings in the jack's name but not suitable for folder's name */
+static const char * const dropped_chars[] = {
+	"/", "=", ",", " ",
+};
+
+static char *strremove(char *s, const char *c)
+{
+	char *p;
+
+	while ((p = strstr(s, c))) {
+		*p = '\0';
+		strcat(s, p+strlen(c));
+	}
+
+	return s;
+}
+
 static int snd_jack_debugfs_add_inject_node(struct snd_jack *jack,
 					    struct snd_jack_kctl *jack_kctl)
 {
 	char *tname;
+	int i;
 
-	/* the folder's name can't contains '/', need to replace it with '!'
-	 * as lib/kobject.c does
-	 */
 	tname = kstrdup(jack_kctl->kctl->id.name, GFP_KERNEL);
 	if (!tname)
 		return -ENOMEM;
-	strreplace(tname, '/', '!');
+
+	for (i = 0; i < ARRAY_SIZE(dropped_chars); i++)
+		tname = strremove(tname, dropped_chars[i]);
+
 	jack_kctl->jack_debugfs_root = debugfs_create_dir(tname, jack->card->debugfs_root);
 	kfree(tname);
 
-- 
2.25.1

Re: [RFC][PATCH v4 2/4] alsa: jack: adjust jack_kctl debugfs folder's name

[Takashi Iwai]

On Mon, 11 Jan 2021 14:05:55 +0100,
Hui Wang wrote:
> 
> We used jack_kctl->kctl.id as the folder's name, but there are some
> characters which are not suitable for foler's name, for example, a
> HDMI/DP audio jack id contains '/', ',', '=' and ' ', this patch will
> remove them from folder's name.
> 
> Before applying patch, the folders look like:
> 'HDMI!DP,pcm=3 Jack'  'Headphone Jack'  'Mic Jack'
> 
> After applying the patch, the folders look like:
> HDMIDPpcm3Jack  HeadphoneJack  MicJack
> 
> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> ---
>  sound/core/jack.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/core/jack.c b/sound/core/jack.c
> index 0092cb6b5b79..e1d1b26f3a5e 100644
> --- a/sound/core/jack.c
> +++ b/sound/core/jack.c
> @@ -243,18 +243,36 @@ static const struct file_operations jackin_inject_fops = {
>  	.llseek = default_llseek,
>  };
>  
> +/* The substrings in the jack's name but not suitable for folder's name */
> +static const char * const dropped_chars[] = {
> +	"/", "=", ",", " ",
> +};
> +
> +static char *strremove(char *s, const char *c)
> +{
> +	char *p;
> +
> +	while ((p = strstr(s, c))) {
> +		*p = '\0';
> +		strcat(s, p+strlen(c));
> +	}
> +
> +	return s;
> +}
> +
>  static int snd_jack_debugfs_add_inject_node(struct snd_jack *jack,
>  					    struct snd_jack_kctl *jack_kctl)
>  {
>  	char *tname;
> +	int i;
>  
> -	/* the folder's name can't contains '/', need to replace it with '!'
> -	 * as lib/kobject.c does
> -	 */
>  	tname = kstrdup(jack_kctl->kctl->id.name, GFP_KERNEL);
>  	if (!tname)
>  		return -ENOMEM;
> -	strreplace(tname, '/', '!');
> +
> +	for (i = 0; i < ARRAY_SIZE(dropped_chars); i++)
> +		tname = strremove(tname, dropped_chars[i]);
> +
>  	jack_kctl->jack_debugfs_root = debugfs_create_dir(tname, jack->card->debugfs_root);
>  	kfree(tname);

We can take a simpler approach, e.g. just replace the chars that are
!isalnum() to '_'.


Takashi

[RFC][PATCH v4 3/4] alsa: jack: add more jack_kctl debugfs nodes

[Hui Wang]

Adding 4 more debugfs nodes, users could get more information about
the jack_kctl from them:
 - kctl_id, read-only, get jack_kctl->kctl's id
   sound/card0/HeadphoneJack# cat kctl_id
   Headphone Jack

 - mask_bits, read-only, get jack_kctl's events mask_bits
   sound/card0/HeadphoneJack# cat mask_bits
   0x0001 HEADPHONE(0x0001)

 - status, read-only, get jack_kctl's current status
   headphone unplugged:
   sound/card0/HeadphoneJack# cat status
   0x0000
   headphone plugged:
   sound/card0/HeadphoneJack# cat status
   0x0001 HEADPHONE(0x0001)

 - type, read-only, get jack's supported events type
   sound/card0/HeadphoneJack# cat type
   0x0001 HEADPHONE(0x0001)

Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 sound/core/jack.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 144 insertions(+)

diff --git a/sound/core/jack.c b/sound/core/jack.c
index e1d1b26f3a5e..fc49dae887f8 100644
--- a/sound/core/jack.c
+++ b/sound/core/jack.c
@@ -230,6 +230,119 @@ static ssize_t jackin_inject_write(struct file *file,
 	return ret;
 }
 
+static ssize_t jack_kctl_id_read(struct file *file,
+				 char __user *to, size_t count, loff_t *ppos)
+{
+	struct snd_jack_kctl *jack_kctl = file->private_data;
+	char *buf;
+	int len, ret;
+
+	buf = kvzalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	len = scnprintf(buf, PAGE_SIZE, "%s\n", jack_kctl->kctl->id.name);
+	ret = simple_read_from_buffer(to, count, ppos, buf, len);
+
+	kvfree(buf);
+	return ret;
+}
+
+/* the bit definition is aligned with snd_jack_types in jack.h */
+static const char * const jack_events_name[] = {
+	"HEADPHONE(0x0001)", "MICROPHONE(0x0002)", "LINEOUT(0x0004)",
+	"MECHANICAL(0x0008)", "VIDEOOUT(0x0010)", "LINEIN(0x0020)",
+	"", "", "", "BTN_5(0x0200)", "BTN_4(0x0400)", "BTN_3(0x0800)",
+	"BTN_2(0x1000)", "BTN_1(0x2000)", "BTN_0(0x4000)", "",
+};
+
+static int parse_mask_bits(unsigned int mask_bits, char *s)
+{
+	char *buf;
+	int len, i;
+
+	buf = kvzalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	len = scnprintf(buf, PAGE_SIZE, "0x%04x", mask_bits);
+
+	for (i = 0; i < 16; i++)
+		if (mask_bits & (1 << i))
+			len += scnprintf(buf + strlen(buf), PAGE_SIZE - strlen(buf),
+					 " %s", jack_events_name[i]);
+
+	len += scnprintf(buf + strlen(buf), PAGE_SIZE - strlen(buf), "\n");
+
+	strcpy(s, buf);
+
+	kvfree(buf);
+
+	return len;
+}
+
+static ssize_t jack_kctl_mask_bits_read(struct file *file,
+					char __user *to, size_t count, loff_t *ppos)
+{
+	struct snd_jack_kctl *jack_kctl = file->private_data;
+	char *buf;
+	int len, ret;
+
+	buf = kvzalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	len = parse_mask_bits(jack_kctl->mask_bits, buf);
+	ret = simple_read_from_buffer(to, count, ppos, buf, len);
+
+	kvfree(buf);
+	return ret;
+}
+
+static ssize_t jack_kctl_status_read(struct file *file,
+				     char __user *to, size_t count, loff_t *ppos)
+{
+	struct snd_jack_kctl *jack_kctl = file->private_data;
+	char *buf;
+	int len, ret;
+
+	buf = kvzalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	len = parse_mask_bits(jack_kctl->kctl->private_value, buf);
+	ret = simple_read_from_buffer(to, count, ppos, buf, len);
+
+	kvfree(buf);
+	return ret;
+}
+
+#ifdef CONFIG_SND_JACK_INPUT_DEV
+static ssize_t jack_type_read(struct file *file,
+			      char __user *to, size_t count, loff_t *ppos)
+{
+	struct snd_jack_kctl *jack_kctl = file->private_data;
+	char *buf;
+	int len, ret;
+
+	buf = kvzalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	len = parse_mask_bits(jack_kctl->jack->type, buf);
+	ret = simple_read_from_buffer(to, count, ppos, buf, len);
+
+	kvfree(buf);
+	return ret;
+}
+
+static const struct file_operations jack_type_fops = {
+	.open = simple_open,
+	.read = jack_type_read,
+	.llseek = default_llseek,
+};
+#endif
+
 static const struct file_operations sw_inject_enable_fops = {
 	.open = simple_open,
 	.read = sw_inject_enable_read,
@@ -243,6 +356,24 @@ static const struct file_operations jackin_inject_fops = {
 	.llseek = default_llseek,
 };
 
+static const struct file_operations jack_kctl_id_fops = {
+	.open = simple_open,
+	.read = jack_kctl_id_read,
+	.llseek = default_llseek,
+};
+
+static const struct file_operations jack_kctl_mask_bits_fops = {
+	.open = simple_open,
+	.read = jack_kctl_mask_bits_read,
+	.llseek = default_llseek,
+};
+
+static const struct file_operations jack_kctl_status_fops = {
+	.open = simple_open,
+	.read = jack_kctl_status_read,
+	.llseek = default_llseek,
+};
+
 /* The substrings in the jack's name but not suitable for folder's name */
 static const char * const dropped_chars[] = {
 	"/", "=", ",", " ",
@@ -282,6 +413,19 @@ static int snd_jack_debugfs_add_inject_node(struct snd_jack *jack,
 	debugfs_create_file("jackin_inject", 0200, jack_kctl->jack_debugfs_root, jack_kctl,
 			    &jackin_inject_fops);
 
+	debugfs_create_file("kctl_id", 0444, jack_kctl->jack_debugfs_root, jack_kctl,
+			    &jack_kctl_id_fops);
+
+	debugfs_create_file("mask_bits", 0444, jack_kctl->jack_debugfs_root, jack_kctl,
+			    &jack_kctl_mask_bits_fops);
+
+	debugfs_create_file("status", 0444, jack_kctl->jack_debugfs_root, jack_kctl,
+			    &jack_kctl_status_fops);
+
+#ifdef CONFIG_SND_JACK_INPUT_DEV
+	debugfs_create_file("type", 0444, jack_kctl->jack_debugfs_root, jack_kctl,
+			    &jack_type_fops);
+#endif
 	return 0;
 }
 #else /* CONFIG_DEBUG_FS */
-- 
2.25.1

Re: [RFC][PATCH v4 3/4] alsa: jack: add more jack_kctl debugfs nodes

[Takashi Iwai]

On Mon, 11 Jan 2021 14:05:56 +0100,
Hui Wang wrote:
> 
> Adding 4 more debugfs nodes, users could get more information about
> the jack_kctl from them:
>  - kctl_id, read-only, get jack_kctl->kctl's id
>    sound/card0/HeadphoneJack# cat kctl_id
>    Headphone Jack
> 
>  - mask_bits, read-only, get jack_kctl's events mask_bits
>    sound/card0/HeadphoneJack# cat mask_bits
>    0x0001 HEADPHONE(0x0001)
> 
>  - status, read-only, get jack_kctl's current status
>    headphone unplugged:
>    sound/card0/HeadphoneJack# cat status
>    0x0000
>    headphone plugged:
>    sound/card0/HeadphoneJack# cat status
>    0x0001 HEADPHONE(0x0001)
> 
>  - type, read-only, get jack's supported events type
>    sound/card0/HeadphoneJack# cat type
>    0x0001 HEADPHONE(0x0001)

I believe we should have a proper documentation in Documentation/ to
describe all those.

And the debugfs callbacks could be optimized without malloc as I
mentioned in another mail, too.


thanks,

Takashi

[RFC][PATCH v4 4/4] alsa: jack: implement save-and-restore for jack's hw status

[Hui Wang]

Once we enable the sw_inject for a jack_kctl, the hw status change
will be blocked, but the hw status still could be reported to
snd_jack_report() and be saved to hw_status_cache.

After the sw_inject is disabled, we use the last saved hw_status_cache
to restore jack's status.

Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 include/sound/jack.h | 1 +
 sound/core/jack.c    | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/include/sound/jack.h b/include/sound/jack.h
index 9eb2b5ec1ec4..1181f536557e 100644
--- a/include/sound/jack.h
+++ b/include/sound/jack.h
@@ -67,6 +67,7 @@ struct snd_jack {
 	char name[100];
 	unsigned int key[6];   /* Keep in sync with definitions above */
 #endif /* CONFIG_SND_JACK_INPUT_DEV */
+	int hw_status_cache;
 	void *private_data;
 	void (*private_free)(struct snd_jack *);
 };
diff --git a/sound/core/jack.c b/sound/core/jack.c
index fc49dae887f8..e15db7894b88 100644
--- a/sound/core/jack.c
+++ b/sound/core/jack.c
@@ -194,8 +194,14 @@ static ssize_t sw_inject_enable_write(struct file *file,
 		goto exit;
 	}
 
+	if (jack_kctl->sw_inject_enable == (!!enable))
+		goto exit;
+
 	jack_kctl->sw_inject_enable = !!enable;
 
+	if (!jack_kctl->sw_inject_enable)
+		snd_jack_report(jack_kctl->jack, jack_kctl->jack->hw_status_cache);
+
  exit:
 	kvfree(buf);
 	return ret;
@@ -679,6 +685,8 @@ void snd_jack_report(struct snd_jack *jack, int status)
 	if (!jack)
 		return;
 
+	jack->hw_status_cache = status;
+
 	list_for_each_entry(jack_kctl, &jack->kctl_list, list)
 		if (jack_kctl->sw_inject_enable)
 			mask_bits |= jack_kctl->mask_bits;
-- 
2.25.1

Re: [RFC][PATCH v4 0/4] design a way to change audio Jack state by software

[Hui Wang]

Hi Takashi,

Looks like no more comment from others, what is your opinion on the 
patchset, is it ok to merge or need to do some change?

Thanks,

Hui.

On 1/11/21 9:05 PM, Hui Wang wrote:
> the changes in the v4:
>   - change the sound-core to sound and change the sound_core_debugfs_root
>     to sound_debugfs_root in the 0001-xxx.patch
>   - change kzalloc/kfree to kvzalloc/kvfree in the debugfs fops for
>     0001-xxx.patch and 0003-xxx.patch
>   - And if needed, I could squash 4 patches into 1 patch before merging.
>
> the changes in the v3 (for easy to review, divide change into 4 patches):
>   - address the comment about the snd_jack_report() and _snd_jack_report(),
>     the v2 design is hard to understand and is hard to review, in the v3,
>     separate the jack_report to snd_jack_report() and snd_jack_inject_report(),
>     hw jack events call snd_jack_report() as before, if a jack contains multi
>     jack_kctl and the jack_kctl's sw_inject is enabled, the status and the
>     related input-dev's events will not be reproted. The injection events call
>     snd_jack_inject_report(). This change is squashed to 0001-xxx.patch,  the
>     rest part of 0001-xxx.patch is same as v2.
>
>   - address the comment about folders'name in the 0002-xxx.patch, so far, drop
>     the '/', ',', '=' and ' ' from the folders' name.
>
>   - address the comment about adding more debugfs nodes in the 0003-xxx.patch,
>     it adds kctl_id, mask_bits, status and type.
>
>   - address the comment about save-n-restore jack's hw status in the
>     0004-xxx.patch, adding a hw_status_cache and save the last reported jack
>     hw event, once the sw_inject is disabled, will restore all jack_kctl's
>     state under the same snd_jack with hw_status_cache.
> [snip]
>
>
> the changes in the V2:
>   - using debugfs instead of sysfs
>   - using jack_ctrl to create a folder instead of snd_jack, since ASoC drivers
>     could create multi jack_ctrls within a snd_jack
>   - create a folder for each jack_ctrl instead for all jack_ctrls
> [ snip ]
>
>
> Hui Wang (4):
>    alsa: jack: implement software jack injection via debugfs
>    alsa: jack: adjust jack_kctl debugfs folder's name
>    alsa: jack: add more jack_kctl debugfs nodes
>    alsa: jack: implement save-and-restore for jack's hw status
>
>   include/sound/core.h |   2 +
>   include/sound/jack.h |   1 +
>   sound/core/init.c    |   7 +
>   sound/core/jack.c    | 352 ++++++++++++++++++++++++++++++++++++++++++-
>   sound/core/sound.c   |   8 +
>   5 files changed, 366 insertions(+), 4 deletions(-)
>

Re: [RFC][PATCH v4 0/4] design a way to change audio Jack state by software

[Takashi Iwai]

On Wed, 20 Jan 2021 09:58:25 +0100,
Hui Wang wrote:
> 
> Hi Takashi,
> 
> Looks like no more comment from others, what is your opinion on the
> patchset, is it ok to merge or need to do some change?

I think the basic concept is OK.  Maybe we want to have a Kconfig to
enable/disable this feature for users who want a slim kernel.

And, it'd be better to reorganize the series.  It's not necessarily
incremental form; e.g. patch 2 doesn't have to be an individual
patch.

I'll take a deeper look for each patch, too.


thanks,

Takashi





> 
> Thanks,
> 
> Hui.
> 
> On 1/11/21 9:05 PM, Hui Wang wrote:
> > the changes in the v4:
> >   - change the sound-core to sound and change the sound_core_debugfs_root
> >     to sound_debugfs_root in the 0001-xxx.patch
> >   - change kzalloc/kfree to kvzalloc/kvfree in the debugfs fops for
> >     0001-xxx.patch and 0003-xxx.patch
> >   - And if needed, I could squash 4 patches into 1 patch before merging.
> >
> > the changes in the v3 (for easy to review, divide change into 4 patches):
> >   - address the comment about the snd_jack_report() and _snd_jack_report(),
> >     the v2 design is hard to understand and is hard to review, in the v3,
> >     separate the jack_report to snd_jack_report() and snd_jack_inject_report(),
> >     hw jack events call snd_jack_report() as before, if a jack contains multi
> >     jack_kctl and the jack_kctl's sw_inject is enabled, the status and the
> >     related input-dev's events will not be reproted. The injection events call
> >     snd_jack_inject_report(). This change is squashed to 0001-xxx.patch,  the
> >     rest part of 0001-xxx.patch is same as v2.
> >
> >   - address the comment about folders'name in the 0002-xxx.patch, so far, drop
> >     the '/', ',', '=' and ' ' from the folders' name.
> >
> >   - address the comment about adding more debugfs nodes in the 0003-xxx.patch,
> >     it adds kctl_id, mask_bits, status and type.
> >
> >   - address the comment about save-n-restore jack's hw status in the
> >     0004-xxx.patch, adding a hw_status_cache and save the last reported jack
> >     hw event, once the sw_inject is disabled, will restore all jack_kctl's
> >     state under the same snd_jack with hw_status_cache.
> > [snip]
> >
> >
> > the changes in the V2:
> >   - using debugfs instead of sysfs
> >   - using jack_ctrl to create a folder instead of snd_jack, since ASoC drivers
> >     could create multi jack_ctrls within a snd_jack
> >   - create a folder for each jack_ctrl instead for all jack_ctrls
> > [ snip ]
> >
> >
> > Hui Wang (4):
> >    alsa: jack: implement software jack injection via debugfs
> >    alsa: jack: adjust jack_kctl debugfs folder's name
> >    alsa: jack: add more jack_kctl debugfs nodes
> >    alsa: jack: implement save-and-restore for jack's hw status
> >
> >   include/sound/core.h |   2 +
> >   include/sound/jack.h |   1 +
> >   sound/core/init.c    |   7 +
> >   sound/core/jack.c    | 352 ++++++++++++++++++++++++++++++++++++++++++-
> >   sound/core/sound.c   |   8 +
> >   5 files changed, 366 insertions(+), 4 deletions(-)
> >
> 

Re: [RFC][PATCH v4 0/4] design a way to change audio Jack state by software

[Hui Wang]

On 1/20/21 10:47 PM, Takashi Iwai wrote:
> On Wed, 20 Jan 2021 09:58:25 +0100,
> Hui Wang wrote:
>> Hi Takashi,
>>
>> Looks like no more comment from others, what is your opinion on the
>> patchset, is it ok to merge or need to do some change?
> I think the basic concept is OK.  Maybe we want to have a Kconfig to
> enable/disable this feature for users who want a slim kernel.
>
> And, it'd be better to reorganize the series.  It's not necessarily
> incremental form; e.g. patch 2 doesn't have to be an individual
> patch.
>
> I'll take a deeper look for each patch, too.
>
>
> thanks,
>
> Takashi
>
Got it, will address all comments, and will send a v5 to review soon.

Thanks,

Hui.

>
>
>
>> Thanks,
>>
>> Hui.
>>
>> On 1/11/21 9:05 PM, Hui Wang wrote:
>>> the changes in the v4:
>>>    - change the sound-core to sound and change the sound_core_debugfs_root
>>>      to sound_debugfs_root in the 0001-xxx.patch
>>>    - change kzalloc/kfree to kvzalloc/kvfree in the debugfs fops for
>>>      0001-xxx.patch and 0003-xxx.patch
>>>    - And if needed, I could squash 4 patches into 1 patch before merging.
>>>
>>> the changes in the v3 (for easy to review, divide change into 4 patches):
>>>    - address the comment about the snd_jack_report() and _snd_jack_report(),
>>>      the v2 design is hard to understand and is hard to review, in the v3,
>>>      separate the jack_report to snd_jack_report() and snd_jack_inject_report(),
>>>      hw jack events call snd_jack_report() as before, if a jack contains multi
>>>      jack_kctl and the jack_kctl's sw_inject is enabled, the status and the
>>>      related input-dev's events will not be reproted. The injection events call
>>>      snd_jack_inject_report(). This change is squashed to 0001-xxx.patch,  the
>>>      rest part of 0001-xxx.patch is same as v2.
>>>
>>>    - address the comment about folders'name in the 0002-xxx.patch, so far, drop
>>>      the '/', ',', '=' and ' ' from the folders' name.
>>>
>>>    - address the comment about adding more debugfs nodes in the 0003-xxx.patch,
>>>      it adds kctl_id, mask_bits, status and type.
>>>
>>>    - address the comment about save-n-restore jack's hw status in the
>>>      0004-xxx.patch, adding a hw_status_cache and save the last reported jack
>>>      hw event, once the sw_inject is disabled, will restore all jack_kctl's
>>>      state under the same snd_jack with hw_status_cache.
>>> [snip]
>>>
>>>
>>> the changes in the V2:
>>>    - using debugfs instead of sysfs
>>>    - using jack_ctrl to create a folder instead of snd_jack, since ASoC drivers
>>>      could create multi jack_ctrls within a snd_jack
>>>    - create a folder for each jack_ctrl instead for all jack_ctrls
>>> [ snip ]
>>>
>>>
>>> Hui Wang (4):
>>>     alsa: jack: implement software jack injection via debugfs
>>>     alsa: jack: adjust jack_kctl debugfs folder's name
>>>     alsa: jack: add more jack_kctl debugfs nodes
>>>     alsa: jack: implement save-and-restore for jack's hw status
>>>
>>>    include/sound/core.h |   2 +
>>>    include/sound/jack.h |   1 +
>>>    sound/core/init.c    |   7 +
>>>    sound/core/jack.c    | 352 ++++++++++++++++++++++++++++++++++++++++++-
>>>    sound/core/sound.c   |   8 +
>>>    5 files changed, 366 insertions(+), 4 deletions(-)
>>>