The various kthreads thread functions (slot_handler_func, sync_func,
recycle_func) in vchiq_core and vchiq_keepalive_thread_func in
vchiq_arm should be stopped when the module is unloaded.
Previous attempt were made to address this but later reverted [1]
due to VC04 firmware corruption. The issue around
wait_event_interruptible() handling on stopping a kthread has been
handled in the previous commit. Hence, it is now safe to stop kthreads
on module unload, without any firmware corruption.
This also completes the "Fix kernel module support" TODO item, hence
drop it from the list.
[1] commit ebee9ca2f59e ("Revert "staging: vc04_services: vchiq_core: Stop kthreads on shutdown"")
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Tested-by: Stefan Wahren <wahrenst@gmx.net>
Link: https://lore.kernel.org/r/20240703131052.597443-3-umang.jain@ideasonboard.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
wait_event_interruptible() returns if the condition evaluates to true
it receives a signal. However, the current code always assume that the
wait_event_interruptible() returns only when the event is fired.
This should not be the case as wait_event_interruptible() can
return on receiving a signal (with -ERESTARTSYS as return value).
We should consider this and bubble up the return value of
wait_event_interruptible() to exactly know if the wait has failed
and error out. This will also help to properly stop kthreads in the
subsequent patch.
Meanwhile at it, remote_wait_event() is modified to return 0 on success,
and an error code (from wait_event_interruptible()) on failure. The
return value is now checked for remote_wait_event() calls.
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Tested-by: Stefan Wahren <wahrenst@gmx.net>
Link: https://lore.kernel.org/r/20240703131052.597443-2-umang.jain@ideasonboard.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Use x instead of x != NULL to improve readability.
Issue identified by checkpatch.
Signed-off-by: Tom Mounet <tommounet@gmail.com>
Reviewed-by: Philipp Hortmann <philipp.g.hortmann@gmail.com>
Link: https://lore.kernel.org/r/66804898.5d0a0220.6df0f.4f0a@mx.google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-----BEGIN PGP SIGNATURE-----
iQFSBAABCAA8FiEEq68RxlopcLEwq+PEeb4+QwBBGIYFAmaB0NweHHRvcnZhbGRz
QGxpbnV4LWZvdW5kYXRpb24ub3JnAAoJEHm+PkMAQRiGkvwH/36UJRk/o6wvXnyH
E6QjCSWo2226APyWks22NjtC3I/8Iqdvkneuh6wG0qL2sXAB078EMjUq5R81bF8H
wWFBJwetjYTp8GEyLioMEb2wCH/J3R29dLFC4UYTplafXRGP6//xcpJaKmTxcgdR
31IzvTPXbApZ7L3k1U6rA2bK9PNKcFCOvZlrNMUCuwMrabymHsDfOUt1DqXyg2xp
zjqiWYBwlklozmgawSWt/mdEgkWuTcAbg+KyqDVQF59s9aj/OOwZ0j+HACq5V8CM
quTPIAYL6CC9p7uxa69lGr/sgC0Is/BZLPX7RTZAwCgarGvnX+1HUsjDcaFCtrVg
O6fPUV8=
=pgUx
-----END PGP SIGNATURE-----
Merge 6.10-rc6 into staging-next
We need the staging driver fixes in here as well.
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Commit 42a2f6664e18 ("staging: vc04_services: Move global g_state to
vchiq_state") adds a parameter to vchiq_debugfs_init, but leaves the
dummy implementation in the !CONFIG_DEBUG_FS case untouched, causing a
compile time error.
Fixes: c3552ab19aeb ("staging: vchiq_debugfs: Fix NPD in vchiq_dump_state")
Signed-off-by: Bernhard Rosenkränzer <bero@baylibre.com>
Reviewed-by: Stefan Wahren <wahrenst@gmx.net>
Link: https://lore.kernel.org/r/20240627124419.2498642-1-bero@baylibre.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The vchiq_state used to be obtained through an accessor which would
validate that the VCHIQ had been initialised correctly with the remote,
or return a null state.
In commit 42a2f6664e18 ("staging: vc04_services: Move global g_state to
vchiq_state") the global state was moved to the vchiq_mgnt structures
stored as a vchiq instance specific context. This conversion removed the
helpers and instead replaced users of this helper with the assumption
that the state is always available and the remote connected.
The conversion does ensure that the state is always available, so some
remaining state null pointer checks that remain are unnecessary, but the
assumption that the remote is present and initialised is incorrect.
Fix this broken assumption by re-introducing the logic that was lost
during the conversion.
Fixes: 42a2f6664e18 ("staging: vc04_services: Move global g_state to vchiq_state")
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Link: https://lore.kernel.org/r/20240620221046.2731704-1-kieran.bingham@ideasonboard.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Since the initial version of the testing instructions a few
things has changed. So consider the latest arm64/defconfig changes
and the new debugfs entry.
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Link: https://lore.kernel.org/r/20240621131958.98208-11-wahrenst@gmx.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The kernel uses different types for DMA length & address,
so better use them.
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Link: https://lore.kernel.org/r/20240621131958.98208-10-wahrenst@gmx.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The number format of VCHIQ debugfs isn't always clear. So let's
add a prefix for all hex values, in order to make things clear.
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Link: https://lore.kernel.org/r/20240621131958.98208-9-wahrenst@gmx.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The service_callback has 5 levels of indentation, which makes it
hard to read. Reduce this by splitting the code in a new function
service_single_message() as suggested by Laurent Pinchart.
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Link: https://lore.kernel.org/r/20240621131958.98208-8-wahrenst@gmx.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
After removal of struct vchiq_2835_state, the init of vchiq_arm_state
can be simplified by doing it directly within vchiq_platform_init_state.
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Link: https://lore.kernel.org/r/20240621131958.98208-7-wahrenst@gmx.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The whole benefit of this encapsulating struct is questionable.
It just stores a flag to signalize the init state of vchiq_arm_state.
Beside the fact this flag is set too soon, the access to uninitialized
members should be avoided. So initialize vchiq_arm_state properly before
assign it directly to vchiq_state.
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Link: https://lore.kernel.org/r/20240621131958.98208-6-wahrenst@gmx.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
These declarations are left from cleanups and not necessary
anymore. So we can drop them.
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Link: https://lore.kernel.org/r/20240621131958.98208-5-wahrenst@gmx.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
There are some struct members, which don't have a real
function. So it's safe to drop them.
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Link: https://lore.kernel.org/r/20240621131958.98208-4-wahrenst@gmx.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The BUG_ON has been replaced with WARN_ON. So we can drop the
comment.
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Link: https://lore.kernel.org/r/20240621131958.98208-3-wahrenst@gmx.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The file vchiq_arm uses a wild mixture of variable names for
return codes. Unify them by using the common name "ret".
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Link: https://lore.kernel.org/r/20240621131958.98208-2-wahrenst@gmx.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
There is a confusing pattern in the kernel to use a variable named
'timeout' to store the result of wait_for_completion_timeout() causing
patterns like:
timeout = wait_for_completion_timeout(...)
if (!timeout) return -ETIMEDOUT;
with all kinds of permutations. Use 'time_left' as a variable to make the
code self explaining.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Reviewed-by: Stefan Wahren <wahrenst@gmx.net>
Link: https://lore.kernel.org/r/20240620182538.5497-2-wsa+renesas@sang-engineering.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
