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
