Hi Kai-Chuan Hsieh,
Is "2e09067 ucm2: Add setting LED Mode in SetLED macro" really
necessary?
If you look closer, the line:
+diff --git a/ucm2/codecs/cs42l45-dmic/init.conf
b/ucm2/codecs/cs42l45-dmic/init.conf
+index 88b19c8..6dd19bb 100644
+--- a/ucm2/codecs/cs42l45-dmic/init.conf
++++ b/ucm2/codecs/cs42l45-dmic/init.conf
+@@ -5,5 +5,5 @@ BootSequence [
+ ]
+
+ Macro [
+- { SetLED { LED="mic" Action="attach" CtlId="cs42l45 FU 113 Mute Switch"
} }
++ { SetLED { LED="mic" Action="attach" CtlId="cs42l45 FU 113 Mute Switch"
Mode="follow-route"} }
+ ]
just gets deleted anyway in "51bada5 ucm2: sof-soundwire: Simplify
cs42l45 configs".
+--- a/ucm2/codecs/cs42l45-dmic/init.conf
++++ b/ucm2/codecs/cs42l45-dmic/init.conf
+
+ Macro [
+- { SetLED { LED="mic" Action="attach" CtlId="cs42l45 FU 113 Mute Switch"
Mode="follow-route"} }
++ { SetLED { LED="mic" Action="attach" CtlId="cs42l45 FU 113 Channel
Switch"} }
+ ]
Whats the point of this patch?
If you look more closely:
+diff --git a/ucm2/common/ctl/led.conf b/ucm2/common/ctl/led.conf
+index f10ad87..7f47daa 100644
+--- a/ucm2/common/ctl/led.conf
++++ b/ucm2/common/ctl/led.conf
+@@ -30,6 +30,7 @@
+ # LED - LED selection - "speaker" or "mic"
+ # Action - action for given control - "attach" or "detach"
+ # CtlId - control identifier (as for amixer, e.g. "Capture Switch" or
"name='CaptureSw
itch',index=1")
++# Mode - (optional) LED mode - "on", "off", "follow-mute" or
"follow-route"
+ #
+
+ DefineMacro.SetLED {
+@@ -46,6 +47,15 @@ DefineMacro.SetLED {
+ }
+ }
+ If.1 {
++ Condition {
++ Type String
++ Empty "${var:-__Mode}"
++ }
++ False.FixedBootSequence [
++ sysw
"-/class/sound/ctl-led/${var:__LED}/mode:${var:__Mode}"
++ ]
++ }
++ If.2 {
+ Condition { Type AlwaysTrue }
+ True.FixedBootSequence [
+ sysw
"-/class/sound/ctl-led/${var:__LED}/card${CardNumber}/${var:__
Action}:${var:__CtlId}"
It moves the existing logic for If.1 into If.2, and adds completely new logic
for If.1.
Is this going to change behaviour of everyones Mute LEDS? How does alsa-ucm-conf
know to use this new If.1 or If.2 for existing behaviour?
I think this doesn't pass the "minimal patch" rules, and creates
unnecessary risk.
Can you just delete the hunk from "51bada5 ucm2: sof-soundwire: Simplify
cs42l45 configs".
instead, and delete "2e09067 ucm2: Add setting LED Mode in SetLED macro"
entirely?
When we backport patches to stable releases, they don't have to always be
straight cherry picks from upstream. Sometimes its better to modify patches so
that we only have the smallest fix possible. Within reason anyway.
That way it would be less likely to regress existing behaviour of the
Mute LED.
PS, your patch format and naming is much better, well done!
Thanks,
Matthew
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/2131725
Title:
Support for Cirrus Logic audio solution CS42L45 with amplifiers on
new Dell PTL platform
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/alsa-ucm-conf/+bug/2131725/+subscriptions
--
ubuntu-bugs mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
