On Thu, Nov 09, 2017 at 04:17:33PM +0000, Mark Brown wrote:
>On Thu, Nov 09, 2017 at 04:09:12PM +0000, Levin, Alexander (Sasha Levin) wrote:
>> On Thu, Nov 09, 2017 at 11:07:44AM +0000, Mark Brown wrote:
>
>> >This doesn't seem like a good idea - note how the changelog says that
>> >the transmit FIFO is at a different address, I'd expect this means that
>> >people won't be able to use the new compatible without a corresponding
>> >code change.
>
>> Interesting. I keep trying to automate DT patch selection to deal with
>> issues as this, but it seems like there is always something...
>
>> I agree with Mark. I think it would make sense to pick 1bd92af877 as
>> well to deal with this issue.
>
>Yeah, if they both go in together it's fine by me (though it's a little
>more than just a device ID).
It's a quirk too :)
--
Thanks,
Sasha
assert_rpm_wakelock_held is triggered from i915_pmic_bus_access_notifier
even though it gets unregistered on (runtime) suspend, this is caused
by a race happening under the following circumstances:
intel_runtime_pm_put does:
atomic_dec(&dev_priv->pm.wakeref_count);
pm_runtime_mark_last_busy(kdev);
pm_runtime_put_autosuspend(kdev);
And pm_runtime_put_autosuspend calls intel_runtime_suspend from
a workqueue, so there is ample of time between the atomic_dec() and
intel_runtime_suspend() unregistering the notifier. If the notifier
gets called in this windowd assert_rpm_wakelock_held falsely triggers
(at this point we're not runtime-suspended yet).
This commit adds disable_rpm_wakeref_asserts and
enable_rpm_wakeref_asserts calls around the
intel_uncore_forcewake_get(FORCEWAKE_ALL) call in
i915_pmic_bus_access_notifier fixing the false-positive WARN_ON.
Changes in v2:
-Reword comment explaining why disabling the wakeref asserts is
ok and necessary
Cc: stable(a)vger.kernel.org
Reported-by: FKr <bugs-freedesktop(a)ubermail.me>
Reviewed-by: Imre Deak <imre.deak(a)intel.com>
Signed-off-by: Hans de Goede <hdegoede(a)redhat.com>
---
drivers/gpu/drm/i915/intel_uncore.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 211acee7c31d..dbc5cc309cbc 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1364,8 +1364,15 @@ static int i915_pmic_bus_access_notifier(struct notifier_block *nb,
* bus, which will be busy after this notification, leading to:
* "render: timed out waiting for forcewake ack request."
* errors.
+ *
+ * The notifier is unregistered during intel_runtime_suspend(),
+ * so it's ok to access the HW here without holding a RPM
+ * wake reference -> disable wakeref asserts for the time of
+ * the access.
*/
+ disable_rpm_wakeref_asserts(dev_priv);
intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+ enable_rpm_wakeref_asserts(dev_priv);
break;
case MBI_PMIC_BUS_ACCESS_END:
intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
--
2.14.3
Hi Greg,
Please consider following upstream selftest firmware changes
to help run mainline/linux-next firmware test scripts on 4.4.y,
without breaking 0-day testing.
47e0bbb7fa98 ("test: firmware_class: report errors properly on failure")
is the one patch actually needed to make sure mainline/linux-next
selftest firmware test scripts run on 4.4.y.
Following commits are, however, required to make sure the above commit
doesn't break 0-day testing, wherein they run in-kernel selftests.
1b1fe542b6f0 ("selftests: firmware: add empty string and async tests")
880444e214cf ("selftests: firmware: send expected errors to /dev/null")
afb999cdef69 ("tools: firmware: check for distro fallback udev cancel rule")
Regards,
Amit Pundir
Brian Norris (2):
test: firmware_class: report errors properly on failure
selftests: firmware: add empty string and async tests
Luis R. Rodriguez (2):
selftests: firmware: send expected errors to /dev/null
tools: firmware: check for distro fallback udev cancel rule
lib/test_firmware.c | 11 ++++++---
tools/testing/selftests/firmware/fw_filesystem.sh | 10 +++++++-
tools/testing/selftests/firmware/fw_userhelper.sh | 28 +++++++++++++++++++++--
3 files changed, 43 insertions(+), 6 deletions(-)
--
2.7.4
This is a note to let you know that I've just added the patch titled
tools: firmware: check for distro fallback udev cancel rule
to the 4.9-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum…
The filename of the patch is:
tools-firmware-check-for-distro-fallback-udev-cancel-rule.patch
and it can be found in the queue-4.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable(a)vger.kernel.org> know about it.
>From afb999cdef69148f366839e74470d8f5375ba5f1 Mon Sep 17 00:00:00 2001
From: "Luis R. Rodriguez" <mcgrof(a)kernel.org>
Date: Mon, 23 Jan 2017 08:11:07 -0800
Subject: tools: firmware: check for distro fallback udev cancel rule
From: Luis R. Rodriguez <mcgrof(a)kernel.org>
commit afb999cdef69148f366839e74470d8f5375ba5f1 upstream.
Some distributions (Debian, OpenSUSE) have a udev rule in place to cancel
all fallback mechanism uevents immediately. This would obviously
make it hard to test against the fallback mechanism test interface,
so we need to check for this.
Signed-off-by: Luis R. Rodriguez <mcgrof(a)kernel.org>
Signed-off-by: Amit Pundir <amit.pundir(a)linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
---
tools/testing/selftests/firmware/fw_userhelper.sh | 28 ++++++++++++++++++++--
1 file changed, 26 insertions(+), 2 deletions(-)
--- a/tools/testing/selftests/firmware/fw_userhelper.sh
+++ b/tools/testing/selftests/firmware/fw_userhelper.sh
@@ -64,9 +64,33 @@ trap "test_finish" EXIT
echo "ABCD0123" >"$FW"
NAME=$(basename "$FW")
+DEVPATH="$DIR"/"nope-$NAME"/loading
+
# Test failure when doing nothing (timeout works).
-echo 1 >/sys/class/firmware/timeout
-echo -n "$NAME" >"$DIR"/trigger_request
+echo -n 2 >/sys/class/firmware/timeout
+echo -n "nope-$NAME" >"$DIR"/trigger_request 2>/dev/null &
+
+# Give the kernel some time to load the loading file, must be less
+# than the timeout above.
+sleep 1
+if [ ! -f $DEVPATH ]; then
+ echo "$0: fallback mechanism immediately cancelled"
+ echo ""
+ echo "The file never appeared: $DEVPATH"
+ echo ""
+ echo "This might be a distribution udev rule setup by your distribution"
+ echo "to immediately cancel all fallback requests, this must be"
+ echo "removed before running these tests. To confirm look for"
+ echo "a firmware rule like /lib/udev/rules.d/50-firmware.rules"
+ echo "and see if you have something like this:"
+ echo ""
+ echo "SUBSYSTEM==\"firmware\", ACTION==\"add\", ATTR{loading}=\"-1\""
+ echo ""
+ echo "If you do remove this file or comment out this line before"
+ echo "proceeding with these tests."
+ exit 1
+fi
+
if diff -q "$FW" /dev/test_firmware >/dev/null ; then
echo "$0: firmware was not expected to match" >&2
exit 1
Patches currently in stable-queue which might be from mcgrof(a)kernel.org are
queue-4.9/tools-firmware-check-for-distro-fallback-udev-cancel-rule.patch
queue-4.9/selftests-firmware-send-expected-errors-to-dev-null.patch
This is a note to let you know that I've just added the patch titled
selftests: firmware: send expected errors to /dev/null
to the 4.9-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum…
The filename of the patch is:
selftests-firmware-send-expected-errors-to-dev-null.patch
and it can be found in the queue-4.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable(a)vger.kernel.org> know about it.
>From 880444e214cfd293a2e8cc4bd3505f7ffa6ce33a Mon Sep 17 00:00:00 2001
From: "Luis R. Rodriguez" <mcgrof(a)kernel.org>
Date: Fri, 16 Dec 2016 03:10:35 -0800
Subject: selftests: firmware: send expected errors to /dev/null
From: Luis R. Rodriguez <mcgrof(a)kernel.org>
commit 880444e214cfd293a2e8cc4bd3505f7ffa6ce33a upstream.
Error that we expect should not be spilled to stdout.
Without this we get:
./fw_filesystem.sh: line 58: printf: write error: Invalid argument
./fw_filesystem.sh: line 63: printf: write error: No such device
./fw_filesystem.sh: line 69: echo: write error: No such file or directory
./fw_filesystem.sh: filesystem loading works
./fw_filesystem.sh: async filesystem loading works
With it:
./fw_filesystem.sh: filesystem loading works
./fw_filesystem.sh: async filesystem loading works
Signed-off-by: Luis R. Rodriguez <mcgrof(a)kernel.org>
Signed-off-by: Amit Pundir <amit.pundir(a)linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
---
tools/testing/selftests/firmware/fw_filesystem.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
--- a/tools/testing/selftests/firmware/fw_filesystem.sh
+++ b/tools/testing/selftests/firmware/fw_filesystem.sh
@@ -48,18 +48,18 @@ echo "ABCD0123" >"$FW"
NAME=$(basename "$FW")
-if printf '\000' >"$DIR"/trigger_request; then
+if printf '\000' >"$DIR"/trigger_request 2> /dev/null; then
echo "$0: empty filename should not succeed" >&2
exit 1
fi
-if printf '\000' >"$DIR"/trigger_async_request; then
+if printf '\000' >"$DIR"/trigger_async_request 2> /dev/null; then
echo "$0: empty filename should not succeed (async)" >&2
exit 1
fi
# Request a firmware that doesn't exist, it should fail.
-if echo -n "nope-$NAME" >"$DIR"/trigger_request; then
+if echo -n "nope-$NAME" >"$DIR"/trigger_request 2> /dev/null; then
echo "$0: firmware shouldn't have loaded" >&2
exit 1
fi
Patches currently in stable-queue which might be from mcgrof(a)kernel.org are
queue-4.9/tools-firmware-check-for-distro-fallback-udev-cancel-rule.patch
queue-4.9/selftests-firmware-send-expected-errors-to-dev-null.patch
This is a note to let you know that I've just added the patch titled
tools: firmware: check for distro fallback udev cancel rule
to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum…
The filename of the patch is:
tools-firmware-check-for-distro-fallback-udev-cancel-rule.patch
and it can be found in the queue-4.4 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable(a)vger.kernel.org> know about it.
>From afb999cdef69148f366839e74470d8f5375ba5f1 Mon Sep 17 00:00:00 2001
From: "Luis R. Rodriguez" <mcgrof(a)kernel.org>
Date: Mon, 23 Jan 2017 08:11:07 -0800
Subject: tools: firmware: check for distro fallback udev cancel rule
From: Luis R. Rodriguez <mcgrof(a)kernel.org>
commit afb999cdef69148f366839e74470d8f5375ba5f1 upstream.
Some distributions (Debian, OpenSUSE) have a udev rule in place to cancel
all fallback mechanism uevents immediately. This would obviously
make it hard to test against the fallback mechanism test interface,
so we need to check for this.
Signed-off-by: Luis R. Rodriguez <mcgrof(a)kernel.org>
Signed-off-by: Amit Pundir <amit.pundir(a)linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
---
tools/testing/selftests/firmware/fw_userhelper.sh | 28 ++++++++++++++++++++--
1 file changed, 26 insertions(+), 2 deletions(-)
--- a/tools/testing/selftests/firmware/fw_userhelper.sh
+++ b/tools/testing/selftests/firmware/fw_userhelper.sh
@@ -64,9 +64,33 @@ trap "test_finish" EXIT
echo "ABCD0123" >"$FW"
NAME=$(basename "$FW")
+DEVPATH="$DIR"/"nope-$NAME"/loading
+
# Test failure when doing nothing (timeout works).
-echo 1 >/sys/class/firmware/timeout
-echo -n "$NAME" >"$DIR"/trigger_request
+echo -n 2 >/sys/class/firmware/timeout
+echo -n "nope-$NAME" >"$DIR"/trigger_request 2>/dev/null &
+
+# Give the kernel some time to load the loading file, must be less
+# than the timeout above.
+sleep 1
+if [ ! -f $DEVPATH ]; then
+ echo "$0: fallback mechanism immediately cancelled"
+ echo ""
+ echo "The file never appeared: $DEVPATH"
+ echo ""
+ echo "This might be a distribution udev rule setup by your distribution"
+ echo "to immediately cancel all fallback requests, this must be"
+ echo "removed before running these tests. To confirm look for"
+ echo "a firmware rule like /lib/udev/rules.d/50-firmware.rules"
+ echo "and see if you have something like this:"
+ echo ""
+ echo "SUBSYSTEM==\"firmware\", ACTION==\"add\", ATTR{loading}=\"-1\""
+ echo ""
+ echo "If you do remove this file or comment out this line before"
+ echo "proceeding with these tests."
+ exit 1
+fi
+
if diff -q "$FW" /dev/test_firmware >/dev/null ; then
echo "$0: firmware was not expected to match" >&2
exit 1
Patches currently in stable-queue which might be from mcgrof(a)kernel.org are
queue-4.4/tools-firmware-check-for-distro-fallback-udev-cancel-rule.patch
queue-4.4/selftests-firmware-send-expected-errors-to-dev-null.patch
This is a note to let you know that I've just added the patch titled
test: firmware_class: report errors properly on failure
to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum…
The filename of the patch is:
test-firmware_class-report-errors-properly-on-failure.patch
and it can be found in the queue-4.4 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable(a)vger.kernel.org> know about it.
>From 47e0bbb7fa985a0f1b5794a8653fae4f8f49de77 Mon Sep 17 00:00:00 2001
From: Brian Norris <computersforpeace(a)gmail.com>
Date: Wed, 9 Dec 2015 14:50:25 -0800
Subject: test: firmware_class: report errors properly on failure
From: Brian Norris <computersforpeace(a)gmail.com>
commit 47e0bbb7fa985a0f1b5794a8653fae4f8f49de77 upstream.
request_firmware() failures currently won't get reported at all (the
error code is discarded). What's more, we get confusing messages, like:
# echo -n notafile > /sys/devices/virtual/misc/test_firmware/trigger_request
[ 8280.311856] test_firmware: loading 'notafile'
[ 8280.317042] test_firmware: load of 'notafile' failed: -2
[ 8280.322445] test_firmware: loaded: 0
# echo $?
0
Report the failures via write() errors, and don't say we "loaded"
anything.
Signed-off-by: Brian Norris <computersforpeace(a)gmail.com>
Acked-by: Kees Cook <keescook(a)chromium.org>
Signed-off-by: Shuah Khan <shuahkh(a)osg.samsung.com>
Signed-off-by: Amit Pundir <amit.pundir(a)linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
---
lib/test_firmware.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
--- a/lib/test_firmware.c
+++ b/lib/test_firmware.c
@@ -65,14 +65,19 @@ static ssize_t trigger_request_store(str
release_firmware(test_firmware);
test_firmware = NULL;
rc = request_firmware(&test_firmware, name, dev);
- if (rc)
+ if (rc) {
pr_info("load of '%s' failed: %d\n", name, rc);
- pr_info("loaded: %zu\n", test_firmware ? test_firmware->size : 0);
+ goto out;
+ }
+ pr_info("loaded: %zu\n", test_firmware->size);
+ rc = count;
+
+out:
mutex_unlock(&test_fw_mutex);
kfree(name);
- return count;
+ return rc;
}
static DEVICE_ATTR_WO(trigger_request);
Patches currently in stable-queue which might be from computersforpeace(a)gmail.com are
queue-4.4/selftests-firmware-add-empty-string-and-async-tests.patch
queue-4.4/test-firmware_class-report-errors-properly-on-failure.patch
This is a note to let you know that I've just added the patch titled
selftests: firmware: send expected errors to /dev/null
to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum…
The filename of the patch is:
selftests-firmware-send-expected-errors-to-dev-null.patch
and it can be found in the queue-4.4 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable(a)vger.kernel.org> know about it.
>From 880444e214cfd293a2e8cc4bd3505f7ffa6ce33a Mon Sep 17 00:00:00 2001
From: "Luis R. Rodriguez" <mcgrof(a)kernel.org>
Date: Fri, 16 Dec 2016 03:10:35 -0800
Subject: selftests: firmware: send expected errors to /dev/null
From: Luis R. Rodriguez <mcgrof(a)kernel.org>
commit 880444e214cfd293a2e8cc4bd3505f7ffa6ce33a upstream.
Error that we expect should not be spilled to stdout.
Without this we get:
./fw_filesystem.sh: line 58: printf: write error: Invalid argument
./fw_filesystem.sh: line 63: printf: write error: No such device
./fw_filesystem.sh: line 69: echo: write error: No such file or directory
./fw_filesystem.sh: filesystem loading works
./fw_filesystem.sh: async filesystem loading works
With it:
./fw_filesystem.sh: filesystem loading works
./fw_filesystem.sh: async filesystem loading works
Signed-off-by: Luis R. Rodriguez <mcgrof(a)kernel.org>
[AmitP: Dropped the async trigger testing parts from original commit]
Signed-off-by: Amit Pundir <amit.pundir(a)linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
---
tools/testing/selftests/firmware/fw_filesystem.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/tools/testing/selftests/firmware/fw_filesystem.sh
+++ b/tools/testing/selftests/firmware/fw_filesystem.sh
@@ -48,13 +48,13 @@ echo "ABCD0123" >"$FW"
NAME=$(basename "$FW")
-if printf '\000' >"$DIR"/trigger_request; then
+if printf '\000' >"$DIR"/trigger_request 2> /dev/null; then
echo "$0: empty filename should not succeed" >&2
exit 1
fi
# Request a firmware that doesn't exist, it should fail.
-if echo -n "nope-$NAME" >"$DIR"/trigger_request; then
+if echo -n "nope-$NAME" >"$DIR"/trigger_request 2> /dev/null; then
echo "$0: firmware shouldn't have loaded" >&2
exit 1
fi
Patches currently in stable-queue which might be from mcgrof(a)kernel.org are
queue-4.4/tools-firmware-check-for-distro-fallback-udev-cancel-rule.patch
queue-4.4/selftests-firmware-send-expected-errors-to-dev-null.patch
This is a note to let you know that I've just added the patch titled
selftests: firmware: add empty string and async tests
to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum…
The filename of the patch is:
selftests-firmware-add-empty-string-and-async-tests.patch
and it can be found in the queue-4.4 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable(a)vger.kernel.org> know about it.
>From 1b1fe542b6f010cf6bc7e1c92805e1c0e133e007 Mon Sep 17 00:00:00 2001
From: Brian Norris <computersforpeace(a)gmail.com>
Date: Wed, 9 Dec 2015 14:50:29 -0800
Subject: selftests: firmware: add empty string and async tests
From: Brian Norris <computersforpeace(a)gmail.com>
commit 1b1fe542b6f010cf6bc7e1c92805e1c0e133e007 upstream.
Now that we've added a 'trigger_async_request' knob to test the
request_firmware_nowait() API, let's use it. Also add tests for the
empty ("") string, since there have been a couple errors in that
handling already.
Since we now have real ways that the sysfs write might fail, let's add
the appropriate check on the 'echo' lines too.
Signed-off-by: Brian Norris <computersforpeace(a)gmail.com>
Acked-by: Kees Cook <keescook(a)chromium.org>
Signed-off-by: Shuah Khan <shuahkh(a)osg.samsung.com>
[AmitP: Dropped the async trigger testing parts from original commit]
Signed-off-by: Amit Pundir <amit.pundir(a)linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
---
tools/testing/selftests/firmware/fw_filesystem.sh | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
--- a/tools/testing/selftests/firmware/fw_filesystem.sh
+++ b/tools/testing/selftests/firmware/fw_filesystem.sh
@@ -48,8 +48,16 @@ echo "ABCD0123" >"$FW"
NAME=$(basename "$FW")
+if printf '\000' >"$DIR"/trigger_request; then
+ echo "$0: empty filename should not succeed" >&2
+ exit 1
+fi
+
# Request a firmware that doesn't exist, it should fail.
-echo -n "nope-$NAME" >"$DIR"/trigger_request
+if echo -n "nope-$NAME" >"$DIR"/trigger_request; then
+ echo "$0: firmware shouldn't have loaded" >&2
+ exit 1
+fi
if diff -q "$FW" /dev/test_firmware >/dev/null ; then
echo "$0: firmware was not expected to match" >&2
exit 1
Patches currently in stable-queue which might be from computersforpeace(a)gmail.com are
queue-4.4/selftests-firmware-add-empty-string-and-async-tests.patch
queue-4.4/test-firmware_class-report-errors-properly-on-failure.patch