#
1cc995a6 |
| 17-Sep-2023 |
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> |
platform/surface: aggregator-cdev: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes many driver authors wrongly assume it's
platform/surface: aggregator-cdev: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes many driver authors wrongly assume it's possible to do error handling by returning an error code. However the value returned is ignored (apart from emitting a warning) and this typically results in resource leaks. To improve here there is a quest to make the remove callback return void. In the first step of this quest all drivers are converted to .remove_new() which already returns void. Eventually after all drivers are converted, .remove_new() is renamed to .remove().
Trivially convert this driver from always returning zero in the remove callback to the void returning variant.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com> Link: https://lore.kernel.org/r/20230917203805.1149595-4-u.kleine-koenig@pengutronix.de Reviewed-by: Hans de Goede <hdegoede@redhat.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
show more ...
|
#
b09ee1cd |
| 20-Dec-2022 |
Maximilian Luz <luzmaximilian@gmail.com> |
platform/surface: aggregator: Rename top-level request functions to avoid ambiguities
We currently have a struct ssam_request_sync and a function ssam_request_sync(). While this is valid C, there ar
platform/surface: aggregator: Rename top-level request functions to avoid ambiguities
We currently have a struct ssam_request_sync and a function ssam_request_sync(). While this is valid C, there are some downsides to it.
One of these is that current Sphinx versions (>= 3.0) cannot disambiguate between the two (see disucssion and pull request linked below). It instead emits a "WARNING: Duplicate C declaration" and links for the struct and function in the resulting documentation link to the same entry (i.e. both to either function or struct documentation) instead of their respective own entries.
While we could just ignore that and wait for a fix, there's also a point to be made that the current naming can be somewhat confusing when searching (e.g. via grep) or trying to understand the levels of abstraction at play:
We currently have struct ssam_request_sync and associated functions ssam_request_sync_[alloc|free|init|wait|...]() operating on this struct. However, function ssam_request_sync() is one abstraction level above this. Similarly, ssam_request_sync_with_buffer() is not a function operating on struct ssam_request_sync, but rather a sibling to ssam_request_sync(), both using the struct under the hood.
Therefore, rename the top level request functions:
ssam_request_sync() -> ssam_request_do_sync() ssam_request_sync_with_buffer() -> ssam_request_do_sync_with_buffer() ssam_request_sync_onstack() -> ssam_request_do_sync_onstack()
Link: https://lore.kernel.org/all/085e0ada65c11da9303d07e70c510dc45f21315b.1656756450.git.mchehab@kernel.org/ Link: https://github.com/sphinx-doc/sphinx/pull/8313 Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com> Link: https://lore.kernel.org/r/20221220175608.1436273-2-luzmaximilian@gmail.com Reviewed-by: Hans de Goede <hdegoede@redhat.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
show more ...
|
#
221756e6 |
| 24-Jun-2022 |
Maximilian Luz <luzmaximilian@gmail.com> |
platform/surface: Update copyright year of various drivers
Update the copyright of various Surface drivers to the current year.
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com> Link: https:/
platform/surface: Update copyright year of various drivers
Update the copyright of various Surface drivers to the current year.
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com> Link: https://lore.kernel.org/r/20220624205800.1355621-4-luzmaximilian@gmail.com Reviewed-by: Hans de Goede <hdegoede@redhat.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
show more ...
|
#
cbd224e0 |
| 04-Jun-2021 |
Maximilian Luz <luzmaximilian@gmail.com> |
platform/surface: aggregator_cdev: Add lockdep support
Mark functions with locking requirements via the corresponding lockdep calls for debugging and documentary purposes.
Signed-off-by: Maximilian
platform/surface: aggregator_cdev: Add lockdep support
Mark functions with locking requirements via the corresponding lockdep calls for debugging and documentary purposes.
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com> Reviewed-by: Hans de Goede <hdegoede@redhat.com> Link: https://lore.kernel.org/r/20210604134755.535590-7-luzmaximilian@gmail.com Signed-off-by: Hans de Goede <hdegoede@redhat.com>
show more ...
|
#
e8e298a6 |
| 04-Jun-2021 |
Maximilian Luz <luzmaximilian@gmail.com> |
platform/surface: aggregator_cdev: Allow enabling of events from user-space
While events can already be enabled and disabled via the generic request IOCTL, this bypasses the internal reference count
platform/surface: aggregator_cdev: Allow enabling of events from user-space
While events can already be enabled and disabled via the generic request IOCTL, this bypasses the internal reference counting mechanism of the controller. Due to that, disabling an event will turn it off regardless of any other client having requested said event, which may break functionality of that client.
To solve this, add IOCTLs wrapping the ssam_controller_event_enable() and ssam_controller_event_disable() functions, which have been previously introduced for this specific purpose.
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com> Reviewed-by: Hans de Goede <hdegoede@redhat.com> Link: https://lore.kernel.org/r/20210604134755.535590-6-luzmaximilian@gmail.com Signed-off-by: Hans de Goede <hdegoede@redhat.com>
show more ...
|
#
776c53c6 |
| 04-Jun-2021 |
Maximilian Luz <luzmaximilian@gmail.com> |
platform/surface: aggregator_cdev: Add support for forwarding events to user-space
Currently, debugging unknown events requires writing a custom driver. This is somewhat difficult, slow to adapt, an
platform/surface: aggregator_cdev: Add support for forwarding events to user-space
Currently, debugging unknown events requires writing a custom driver. This is somewhat difficult, slow to adapt, and not entirely user-friendly for quickly trying to figure out things on devices of some third-party user. We can do better. We already have a user-space interface intended for debugging SAM EC requests, so let's add support for receiving events to that.
This commit provides support for receiving events by reading from the controller file. It additionally introduces two new IOCTLs to control which event categories will be forwarded. Specifically, a user-space client can specify which target categories it wants to receive events from by registering the corresponding notifier(s) via the IOCTLs and after that, read the received events by reading from the controller device.
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com> Reviewed-by: Hans de Goede <hdegoede@redhat.com> Link: https://lore.kernel.org/r/20210604134755.535590-5-luzmaximilian@gmail.com Signed-off-by: Hans de Goede <hdegoede@redhat.com>
show more ...
|
#
e94a2650 |
| 11-Jan-2021 |
Maximilian Luz <luzmaximilian@gmail.com> |
platform/surface: aggregator_cdev: Add comments regarding unchecked allocation size
CI static analysis complains about the allocation size in payload and response buffers being unchecked. In general
platform/surface: aggregator_cdev: Add comments regarding unchecked allocation size
CI static analysis complains about the allocation size in payload and response buffers being unchecked. In general, these allocations should be safe as the user-input is u16 and thus limited to U16_MAX, which is only slightly larger than the theoretical maximum imposed by the underlying SSH protocol.
All bounds on these values required by the underlying protocol are enforced in ssam_request_sync() (or rather the functions called by it), thus bounds here are only relevant for allocation.
Add comments explaining that this should be safe.
Reported-by: Colin Ian King <colin.king@canonical.com> Fixes: 178f6ab77e61 ("platform/surface: Add Surface Aggregator user-space interface") Addresses-Coverity: ("Untrusted allocation size") Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com> Link: https://lore.kernel.org/r/20210111154851.325404-3-luzmaximilian@gmail.com Signed-off-by: Hans de Goede <hdegoede@redhat.com>
show more ...
|
#
a403c1df |
| 11-Jan-2021 |
Maximilian Luz <luzmaximilian@gmail.com> |
platform/surface: aggregator_cdev: Fix access of uninitialized variables
When copy_struct_from_user() in ssam_cdev_request() fails, we directly jump to the 'out' label. In this case, however 'spec'
platform/surface: aggregator_cdev: Fix access of uninitialized variables
When copy_struct_from_user() in ssam_cdev_request() fails, we directly jump to the 'out' label. In this case, however 'spec' and 'rsp' are not initialized, but we still access fields of those variables. Fix this by initializing them at the time of their declaration.
Reported-by: Colin Ian King <colin.king@canonical.com> Fixes: 178f6ab77e61 ("platform/surface: Add Surface Aggregator user-space interface") Addresses-Coverity: ("Uninitialized pointer read") Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com> Link: https://lore.kernel.org/r/20210111154851.325404-2-luzmaximilian@gmail.com Signed-off-by: Hans de Goede <hdegoede@redhat.com>
show more ...
|
#
178f6ab7 |
| 21-Dec-2020 |
Maximilian Luz <luzmaximilian@gmail.com> |
platform/surface: Add Surface Aggregator user-space interface
Add a misc-device providing user-space access to the Surface Aggregator EC, mainly intended for debugging, testing, and reverse-engineer
platform/surface: Add Surface Aggregator user-space interface
Add a misc-device providing user-space access to the Surface Aggregator EC, mainly intended for debugging, testing, and reverse-engineering. This interface gives user-space applications the ability to send requests to the EC and receive the corresponding responses.
The device-file is managed by a pseudo platform-device and corresponding driver to avoid dependence on the dedicated bus, allowing it to be loaded in a minimal configuration.
A python library and scripts to access this device can be found at [1].
[1]: https://github.com/linux-surface/surface-aggregator-module/tree/master/scripts/ssam
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com> Link: https://lore.kernel.org/r/20201221183959.1186143-9-luzmaximilian@gmail.com Signed-off-by: Hans de Goede <hdegoede@redhat.com>
show more ...
|