[RFC][PATCH v2 0/1] design a way to change audio Jack state by software
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
This is the layout of folder on a machine with 2 sound cards: root@hwang4-ThinkPad-P1-Gen-3:/sys/kernel/debug# tree /sys/kernel/debug/sound-core/ /sys/kernel/debug/sound-core/ ├── 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
The sw_inject_enable has rw mode, the jackin_inject has write_only mode.
If users want to inject a plugin to Headphone Jack, they could run: echo 1 > 'Headphone Jack'/sw_inject_enable /* now, this jack state can't be changed by hw events */ cat 'Headphone Jack'/sw_inject_enable ack: Headphone Jack Inject Enabled: 1 echo 1 > 'Headphone Jack'/jackin_inject /* this will inject a plugin to Headphone Jack */
After testing, run echo 0 > 'Headphone Jack'/sw_inject_enable /* the hw events will change the jack state */
Hui Wang (1): alsa: jack: implement software jack injection via debugfs
include/sound/core.h | 2 + sound/core/init.c | 7 ++ sound/core/jack.c | 202 ++++++++++++++++++++++++++++++++++++------- sound/core/sound.c | 8 ++ 4 files changed, 188 insertions(+), 31 deletions(-)
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-core root folder in the debugfs dir, and each sound card will create a folder cardN under sound-core, 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-core sound-core/ ├── 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.
Signed-off-by: Hui Wang hui.wang@canonical.com --- include/sound/core.h | 2 + sound/core/init.c | 7 ++ sound/core/jack.c | 202 ++++++++++++++++++++++++++++++++++++------- sound/core/sound.c | 8 ++ 4 files changed, 188 insertions(+), 31 deletions(-)
diff --git a/include/sound/core.h b/include/sound/core.h index 0462c577d7a3..11d61e4248ee 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_core_debugfs_root;
void snd_request_card(int card);
diff --git a/sound/core/init.c b/sound/core/init.c index 764dbe673d48..a9ceaf788019 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> @@ -163,6 +164,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; @@ -246,6 +248,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_core_debugfs_root); + *card_ret = card; return 0;
@@ -418,6 +424,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..fa6b3cdc0431 100644 --- a/sound/core/jack.c +++ b/sound/core/jack.c @@ -8,6 +8,8 @@ #include <linux/input.h> #include <linux/slab.h> #include <linux/module.h> +#include <linux/ctype.h> +#include <linux/debugfs.h> #include <sound/jack.h> #include <sound/core.h> #include <sound/control.h> @@ -16,6 +18,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 +114,174 @@ static int snd_jack_dev_register(struct snd_device *device) } #endif /* CONFIG_SND_JACK_INPUT_DEV */
+static void _snd_jack_report(struct snd_jack *jack, int status, bool from_inject) +{ + struct snd_jack_kctl *jack_kctl; + unsigned int mask_bits = 0; +#ifdef CONFIG_SND_JACK_INPUT_DEV + int i; +#endif + list_for_each_entry(jack_kctl, &jack->kctl_list, list) { + if (jack_kctl->sw_inject_enable == from_inject) + snd_kctl_jack_report(jack->card, jack_kctl->kctl, + status & jack_kctl->mask_bits); + else if (jack_kctl->sw_inject_enable) + mask_bits |= 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; + + if (!from_inject) + testbit &= ~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; + + if (!from_inject) + testbit &= ~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 */ +} + +#ifdef CONFIG_DEBUG_FS +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 = kzalloc(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); + + kfree(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 = kzalloc(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: + kfree(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 = kzalloc(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_report(jack_kctl->jack, !!enable ? jack_kctl->mask_bits : 0, true); + + exit: + kfree(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 +static int snd_jack_debugfs_add_inject_node(struct snd_jack *jack, + struct snd_jack_kctl *jack_kctl) +{ + return 0; +} +#endif + 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 +289,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) @@ -339,39 +509,9 @@ EXPORT_SYMBOL(snd_jack_set_key); */ void snd_jack_report(struct snd_jack *jack, int status) { - struct snd_jack_kctl *jack_kctl; -#ifdef CONFIG_SND_JACK_INPUT_DEV - int i; -#endif - if (!jack) 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); - -#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; - - 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; - 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 */ + return _snd_jack_report(jack, status, false); } EXPORT_SYMBOL(snd_jack_report); diff --git a/sound/core/sound.c b/sound/core/sound.c index b75f78f2c4b8..3039e317df93 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_core_debugfs_root; +EXPORT_SYMBOL_GPL(sound_core_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_core_debugfs_root = debugfs_create_dir("sound-core", 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_core_debugfs_root); snd_info_done(); unregister_chrdev(major, "alsa"); }
Hey,
I gave a quick test spin and features seems to work as advertized. A few minor comments on the code. If Jaroslav you think this would be ok as an approach, I can give a more extensive test run on this.
On Wed, 16 Dec 2020, Hui Wang wrote:
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-core root folder in the debugfs dir, and each sound card will create a folder cardN under sound-core, 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-core sound-core/ ├── card0 │ ├── HDMI!DP,pcm=10 Jack
this combination of "!,= " characters in filenames is a bit non-unixy, but maybe in 2020 we are ready for this.
+static void _snd_jack_report(struct snd_jack *jack, int status, bool from_inject) +{
- struct snd_jack_kctl *jack_kctl;
- unsigned int mask_bits = 0;
+#ifdef CONFIG_SND_JACK_INPUT_DEV
- int i;
+#endif
- list_for_each_entry(jack_kctl, &jack->kctl_list, list) {
if (jack_kctl->sw_inject_enable == from_inject)snd_kctl_jack_report(jack->card, jack_kctl->kctl,status & jack_kctl->mask_bits);else if (jack_kctl->sw_inject_enable)mask_bits |= jack_kctl->mask_bits;- }
I'm wondering if it would be worth the code duplication to have the inject-variant of this code in a separate function. I find the above code to set up "mask_bits" a bit hard to read and this adds a layer of complexity to anyone just wanting to look at the regular jack report code path.
+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 = kzalloc(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;
Here it's a bit annoying that after you disable sw_inject, the kcontrol values are not restored to reflrect actual hw state (until there are new jack events from hw). User-space cannot completely handle the save'n'restore as it cannot detect if real hw jack status changed during the sw-inject test. OTOH, this would require caching the most recent value and maybe not worth the effort.
+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);
This goes over 100-column limit and triggers a checkpatch complaint.
Br, Kai
On Thu, 17 Dec 2020 17:45:05 +0100, Kai Vehmanen wrote:
Hey,
I gave a quick test spin and features seems to work as advertized. A few minor comments on the code. If Jaroslav you think this would be ok as an approach, I can give a more extensive test run on this.
The tree representation looks better than the previous one, IMO. The exact contents would need more brush up, though; e.g. the content of each jack could be shown in a debugfs node as well as the injection. Or the type and the mask-to-be-injected can be shown there, too.
+static void _snd_jack_report(struct snd_jack *jack, int status, bool from_inject) +{
- struct snd_jack_kctl *jack_kctl;
- unsigned int mask_bits = 0;
+#ifdef CONFIG_SND_JACK_INPUT_DEV
- int i;
+#endif
- list_for_each_entry(jack_kctl, &jack->kctl_list, list) {
if (jack_kctl->sw_inject_enable == from_inject)snd_kctl_jack_report(jack->card, jack_kctl->kctl,status & jack_kctl->mask_bits);else if (jack_kctl->sw_inject_enable)mask_bits |= jack_kctl->mask_bits;- }
I'm wondering if it would be worth the code duplication to have the inject-variant of this code in a separate function. I find the above code to set up "mask_bits" a bit hard to read and this adds a layer of complexity to anyone just wanting to look at the regular jack report code path.
Yes, that's my impression, too. The logic is hard to follow.
+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 = kzalloc(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;
Here it's a bit annoying that after you disable sw_inject, the kcontrol values are not restored to reflrect actual hw state (until there are new jack events from hw). User-space cannot completely handle the save'n'restore as it cannot detect if real hw jack status changed during the sw-inject test. OTOH, this would require caching the most recent value and maybe not worth the effort.
Right, but I guess this can be ignored.
Or, as I mentioned in the above, we may expose the current value in each node instead, and writing a value to this node is treated as injection. Then the rest requirement is rather masking from the hardware update.
thanks,
Takashi
On 12/18/20 11:17 PM, Takashi Iwai wrote:
On Thu, 17 Dec 2020 17:45:05 +0100, Kai Vehmanen wrote:
Hey,
I gave a quick test spin and features seems to work as advertized. A few minor comments on the code. If Jaroslav you think this would be ok as an approach, I can give a more extensive test run on this.
The tree representation looks better than the previous one, IMO. The exact contents would need more brush up, though; e.g. the content of each jack could be shown in a debugfs node as well as the injection. Or the type and the mask-to-be-injected can be shown there, too.
OK, got it, will add more nodes for a jack, the nodes will bring more info of the jack to the userspace.
+static void _snd_jack_report(struct snd_jack *jack, int status, bool from_inject) +{
- struct snd_jack_kctl *jack_kctl;
- unsigned int mask_bits = 0;
+#ifdef CONFIG_SND_JACK_INPUT_DEV
- int i;
+#endif
- list_for_each_entry(jack_kctl, &jack->kctl_list, list) {
if (jack_kctl->sw_inject_enable == from_inject)snd_kctl_jack_report(jack->card, jack_kctl->kctl,status & jack_kctl->mask_bits);else if (jack_kctl->sw_inject_enable)mask_bits |= jack_kctl->mask_bits;- }
I'm wondering if it would be worth the code duplication to have the inject-variant of this code in a separate function. I find the above code to set up "mask_bits" a bit hard to read and this adds a layer of complexity to anyone just wanting to look at the regular jack report code path.
Yes, that's my impression, too. The logic is hard to follow.
I think it is really complicated, That is my design:
- If a jack_ctrl's sw_inject is enabled, the jack_report will only report status from injection (block hw events), if it is disabled, the jack_report will only report status from hw events (block injection). That is why I have to add a parameter from_inject
- A snd_jack could contain multi jack_ctrls, the snd_jack_report(status) is based on snd_jack instead of jack_ctrls, but sw_inject_enable is based on jack_ctrls instead of snd_jack. Suppose a snd_jack has 2 jack_ctrls A and B, A's sw_inject is enabled. Suppose Jack of A triggers a hw events and snd_jack_report() is called, the status should be blocked since A's sw_inject is enabled, also the input_dev's event of this jack_ctrl should be blocked too, I added mask_bits |= jack_kctl->mask_bits just for blocking the input-dev's report.
So far, I could not design a cleaner and simpler function to implement the idea above.
+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 = kzalloc(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;
Here it's a bit annoying that after you disable sw_inject, the kcontrol values are not restored to reflrect actual hw state (until there are new jack events from hw). User-space cannot completely handle the save'n'restore as it cannot detect if real hw jack status changed during the sw-inject test. OTOH, this would require caching the most recent value and maybe not worth the effort.
Right, but I guess this can be ignored.
Or, as I mentioned in the above, we may expose the current value in each node instead, and writing a value to this node is treated as injection. Then the rest requirement is rather masking from the hardware update.
Also, I could add a hw_status_cache in the snd_jack_kctl{}, and use it to implement save-and-restore for the jack's state.
Thanks.
thanks,
Takashi
On 12/18/20 12:45 AM, Kai Vehmanen wrote:
Hey,
I gave a quick test spin and features seems to work as advertized. A few minor comments on the code. If Jaroslav you think this would be ok as an approach, I can give a more extensive test run on this.
[snip]
sound-core/ ├── card0 │ ├── HDMI!DP,pcm=10 Jack this combination of "!,= " characters in filenames is a bit non-unixy, but maybe in 2020 we are ready for this.
OK, will try to remove those characters.
+static void _snd_jack_report(struct snd_jack *jack, int status, bool from_inject) +{
- struct snd_jack_kctl *jack_kctl;
[snip]
- 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);
This goes over 100-column limit and triggers a checkpatch complaint.
Got it, will fix it.
Thanks.
Br, Kai
participants (3)
-
Hui Wang -
Kai Vehmanen -
Takashi Iwai