* [PATCH] alsa-restore.rules: refer to correct attr
@ 2014-01-12 16:15 Dave Reisner
2014-01-12 16:39 ` Dave Reisner
0 siblings, 1 reply; 3+ messages in thread
From: Dave Reisner @ 2014-01-12 16:15 UTC (permalink / raw)
To: alsa-devel; +Cc: Dave Reisner
$attr{number} in the RUN rule is an empty expansion. This makes sense,
because the path doesn't exist -- i.e., it refers to the path:
/sys/devices/pci0000:00/foo/bar/sound/card0/controlC0/number
Instead, refer to $attr{device/number}, which does exist.
Signed-off-by: Dave Reisner <dreisner@archlinux.org>
---
This seems to have been broken when it was first introduced a little over
three years ago (de7c3eff0e).
Please CC me on replies as I am not subscribed to the list.
alsactl/90-alsa-restore.rules.in | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/alsactl/90-alsa-restore.rules.in b/alsactl/90-alsa-restore.rules.in
index 88e12e0..c68119d 100644
--- a/alsactl/90-alsa-restore.rules.in
+++ b/alsactl/90-alsa-restore.rules.in
@@ -2,7 +2,7 @@ ACTION=="add", SUBSYSTEM=="sound", KERNEL=="controlC*", KERNELS!="card*", GOTO="
GOTO="alsa_restore_end"
LABEL="alsa_restore_go"
-TEST!="@daemonswitch@", RUN+="@sbindir@/alsactl restore $attr{number}"
-TEST=="@daemonswitch@", RUN+="@sbindir@/alsactl nrestore $attr{number}"
+TEST!="@daemonswitch@", RUN+="@sbindir@/alsactl restore $attr{device/number}"
+TEST=="@daemonswitch@", RUN+="@sbindir@/alsactl nrestore $attr{device/number}"
LABEL="alsa_restore_end"
--
1.8.5.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] alsa-restore.rules: refer to correct attr 2014-01-12 16:15 [PATCH] alsa-restore.rules: refer to correct attr Dave Reisner @ 2014-01-12 16:39 ` Dave Reisner 2014-01-13 10:31 ` [alsa-devel] " Takashi Iwai 0 siblings, 1 reply; 3+ messages in thread From: Dave Reisner @ 2014-01-12 16:39 UTC (permalink / raw) To: Dave Reisner; +Cc: alsa-devel, systemd-devel On Sun, Jan 12, 2014 at 11:15:52AM -0500, Dave Reisner wrote: > $attr{number} in the RUN rule is an empty expansion. This makes sense, > because the path doesn't exist -- i.e., it refers to the path: > > /sys/devices/pci0000:00/foo/bar/sound/card0/controlC0/number > > Instead, refer to $attr{device/number}, which does exist. > > Signed-off-by: Dave Reisner <dreisner@archlinux.org> > --- > This seems to have been broken when it was first introduced a little over > three years ago (de7c3eff0e). I dug into this a little more to figure out why this potentially *would* have worked and found the following advice from udev(7) about $attr{}: If the matching device does not have such an attribute, and a previous KERNELS, SUBSYSTEMS, DRIVERS, or ATTRS test selected a parent device, then the attribute from that parent device is used. So, knowing how RUN expansions work, the simple presence of the following rule causes $attr{number} to expand properly SUBSYSTEM=="sound", ATTRS{id}=="M2496" I named this /etc/udev/rules/91-alsa-foo.rules -- intentionally after the restore rule. Of course, this specifically matches my system, but one could imagine a more general rule which would cause parent matches and thus change the behavior of the subsequent $attr{number} expansion. I'm not sure if this is intended behavior in udev since the parent match doesn't occur in the same rule. So, $attr{number} is potentially correct (but this seems weird), while $attr{device/number} should always be correct. Hope this makes sense... > Please CC me on replies as I am not subscribed to the list. > > alsactl/90-alsa-restore.rules.in | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/alsactl/90-alsa-restore.rules.in b/alsactl/90-alsa-restore.rules.in > index 88e12e0..c68119d 100644 > --- a/alsactl/90-alsa-restore.rules.in > +++ b/alsactl/90-alsa-restore.rules.in > @@ -2,7 +2,7 @@ ACTION=="add", SUBSYSTEM=="sound", KERNEL=="controlC*", KERNELS!="card*", GOTO=" > GOTO="alsa_restore_end" > > LABEL="alsa_restore_go" > -TEST!="@daemonswitch@", RUN+="@sbindir@/alsactl restore $attr{number}" > -TEST=="@daemonswitch@", RUN+="@sbindir@/alsactl nrestore $attr{number}" > +TEST!="@daemonswitch@", RUN+="@sbindir@/alsactl restore $attr{device/number}" > +TEST=="@daemonswitch@", RUN+="@sbindir@/alsactl nrestore $attr{device/number}" > > LABEL="alsa_restore_end" > -- > 1.8.5.2 > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [alsa-devel] [PATCH] alsa-restore.rules: refer to correct attr 2014-01-12 16:39 ` Dave Reisner @ 2014-01-13 10:31 ` Takashi Iwai 0 siblings, 0 replies; 3+ messages in thread From: Takashi Iwai @ 2014-01-13 10:31 UTC (permalink / raw) To: Dave Reisner; +Cc: Dave Reisner, systemd-devel, alsa-devel At Sun, 12 Jan 2014 11:39:29 -0500, Dave Reisner wrote: > > On Sun, Jan 12, 2014 at 11:15:52AM -0500, Dave Reisner wrote: > > $attr{number} in the RUN rule is an empty expansion. This makes sense, > > because the path doesn't exist -- i.e., it refers to the path: > > > > /sys/devices/pci0000:00/foo/bar/sound/card0/controlC0/number > > > > Instead, refer to $attr{device/number}, which does exist. > > > > Signed-off-by: Dave Reisner <dreisner@archlinux.org> > > --- > > This seems to have been broken when it was first introduced a little over > > three years ago (de7c3eff0e). > > I dug into this a little more to figure out why this potentially *would* > have worked and found the following advice from udev(7) about $attr{}: > > If the matching device does not have such an attribute, and a previous > KERNELS, SUBSYSTEMS, DRIVERS, or ATTRS test selected a parent device, > then the attribute from that parent device is used. > > So, knowing how RUN expansions work, the simple presence of the > following rule causes $attr{number} to expand properly > > SUBSYSTEM=="sound", ATTRS{id}=="M2496" > > I named this /etc/udev/rules/91-alsa-foo.rules -- intentionally after > the restore rule. > > Of course, this specifically matches my system, but one could imagine > a more general rule which would cause parent matches and thus change the > behavior of the subsequent $attr{number} expansion. I'm not sure if this > is intended behavior in udev since the parent match doesn't occur in the > same rule. > > So, $attr{number} is potentially correct (but this seems weird), while > $attr{device/number} should always be correct. > > Hope this makes sense... Yeah, $attr{device/number} looks definitely better. I applied the patch now. Thanks. Takashi > > > Please CC me on replies as I am not subscribed to the list. > > > > alsactl/90-alsa-restore.rules.in | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/alsactl/90-alsa-restore.rules.in b/alsactl/90-alsa-restore.rules.in > > index 88e12e0..c68119d 100644 > > --- a/alsactl/90-alsa-restore.rules.in > > +++ b/alsactl/90-alsa-restore.rules.in > > @@ -2,7 +2,7 @@ ACTION=="add", SUBSYSTEM=="sound", KERNEL=="controlC*", KERNELS!="card*", GOTO=" > > GOTO="alsa_restore_end" > > > > LABEL="alsa_restore_go" > > -TEST!="@daemonswitch@", RUN+="@sbindir@/alsactl restore $attr{number}" > > -TEST=="@daemonswitch@", RUN+="@sbindir@/alsactl nrestore $attr{number}" > > +TEST!="@daemonswitch@", RUN+="@sbindir@/alsactl restore $attr{device/number}" > > +TEST=="@daemonswitch@", RUN+="@sbindir@/alsactl nrestore $attr{device/number}" > > > > LABEL="alsa_restore_end" > > -- > > 1.8.5.2 > > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-01-13 10:31 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-01-12 16:15 [PATCH] alsa-restore.rules: refer to correct attr Dave Reisner 2014-01-12 16:39 ` Dave Reisner 2014-01-13 10:31 ` [alsa-devel] " Takashi Iwai
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.