Re: [PATCH] platform/surface: aggregator: Make SSAM_DEFINE_SYNC_REQUEST_x define static functions
Hi,
On 3/4/21 8:05 PM, Maximilian Luz wrote:
> The SSAM_DEFINE_SYNC_REQUEST_x() macros are intended to reduce
> boiler-plate code for SSAM request definitions by defining a wrapper
> function for the specified request. The client device variants of those
> macros, i.e. SSAM_DEFINE_SYNC_REQUEST_CL_x() in particular rely on the
> multi-device (MD) variants, e.g.:
>
> #define SSAM_DEFINE_SYNC_REQUEST_CL_R(name, rtype, spec...) \
> SSAM_DEFINE_SYNC_REQUEST_MD_R(__raw_##name, rtype, spec) \
> int name(struct ssam_device *sdev, rtype *ret)\
> { \
> return __raw_##name(sdev->ctrl, sdev->uid.target, \
> sdev->uid.instance, ret); \
> }
>
> This now creates the problem that it is not possible to declare the
> generated functions static via
>
> static SSAM_DEFINE_SYNC_REQUEST_CL_R(...)
>
> as this will only apply to the function defined by the multi-device
> macro, i.e. SSAM_DEFINE_SYNC_REQUEST_MD_R(). Thus compiling with
> `-Wmissing-prototypes' rightfully complains that there is a 'static'
> keyword missing.
>
> To solve this, make all SSAM_DEFINE_SYNC_REQUEST_x() macros define
> static functions. Non-client-device macros are also changed for
> consistency. In general, we expect those functions to be only used
> locally in the respective drivers for the corresponding interfaces, so
> having to define a wrapper function to be able to export this should be
> the odd case out.
>
> Reported-by: kernel test robot
> Fixes: 510c8114fc74 ("platform/surface: Add platform profile driver")
> Signed-off-by: Maximilian Luz
Thank you for the quick fix.
I've applied this patch to my review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
> ---
> .../driver-api/surface_aggregator/client.rst | 4 +-
> .../platform/surface/aggregator/controller.c | 10 +--
> .../surface/surface_aggregator_registry.c | 2 +-
> .../surface/surface_platform_profile.c| 4 +-
> include/linux/surface_aggregator/controller.h | 74 +--
> include/linux/surface_aggregator/device.h | 31
> 6 files changed, 63 insertions(+), 62 deletions(-)
>
> diff --git a/Documentation/driver-api/surface_aggregator/client.rst
> b/Documentation/driver-api/surface_aggregator/client.rst
> index 26d13085a117..e519d374c378 100644
> --- a/Documentation/driver-api/surface_aggregator/client.rst
> +++ b/Documentation/driver-api/surface_aggregator/client.rst
> @@ -248,7 +248,7 @@ This example defines a function
>
> .. code-block:: c
>
> - int __ssam_tmp_perf_mode_set(struct ssam_controller *ctrl, const __le32
> *arg);
> + static int __ssam_tmp_perf_mode_set(struct ssam_controller *ctrl, const
> __le32 *arg);
>
> executing the specified request, with the controller passed in when calling
> said function. In this example, the argument is provided via the ``arg``
> @@ -296,7 +296,7 @@ This invocation of the macro defines a function
>
> .. code-block:: c
>
> - int ssam_bat_get_sta(struct ssam_device *sdev, __le32 *ret);
> + static int ssam_bat_get_sta(struct ssam_device *sdev, __le32 *ret);
>
> executing the specified request, using the device IDs and controller given
> in the client device. The full list of such macros for client devices is:
> diff --git a/drivers/platform/surface/aggregator/controller.c
> b/drivers/platform/surface/aggregator/controller.c
> index 5bcb59ed579d..aa6f37b4f46e 100644
> --- a/drivers/platform/surface/aggregator/controller.c
> +++ b/drivers/platform/surface/aggregator/controller.c
> @@ -1750,35 +1750,35 @@ EXPORT_SYMBOL_GPL(ssam_request_sync_with_buffer);
>
> /* -- Internal SAM requests.
> */
>
> -static SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_get_firmware_version, __le32, {
> +SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_get_firmware_version, __le32, {
> .target_category = SSAM_SSH_TC_SAM,
> .target_id = 0x01,
> .command_id = 0x13,
> .instance_id = 0x00,
> });
>
> -static SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_display_off, u8, {
> +SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_display_off, u8, {
> .target_category = SSAM_SSH_TC_SAM,
> .target_id = 0x01,
> .command_id = 0x15,
> .instance_id = 0x00,
> });
>
> -static SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_display_on, u8, {
> +SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_display_on, u8, {
> .target_category = SSAM_SSH_TC_SAM,
>
[PATCH] platform/surface: aggregator: Make SSAM_DEFINE_SYNC_REQUEST_x define static functions
The SSAM_DEFINE_SYNC_REQUEST_x() macros are intended to reduce
boiler-plate code for SSAM request definitions by defining a wrapper
function for the specified request. The client device variants of those
macros, i.e. SSAM_DEFINE_SYNC_REQUEST_CL_x() in particular rely on the
multi-device (MD) variants, e.g.:
#define SSAM_DEFINE_SYNC_REQUEST_CL_R(name, rtype, spec...) \
SSAM_DEFINE_SYNC_REQUEST_MD_R(__raw_##name, rtype, spec) \
int name(struct ssam_device *sdev, rtype *ret)\
{ \
return __raw_##name(sdev->ctrl, sdev->uid.target, \
sdev->uid.instance, ret); \
}
This now creates the problem that it is not possible to declare the
generated functions static via
static SSAM_DEFINE_SYNC_REQUEST_CL_R(...)
as this will only apply to the function defined by the multi-device
macro, i.e. SSAM_DEFINE_SYNC_REQUEST_MD_R(). Thus compiling with
`-Wmissing-prototypes' rightfully complains that there is a 'static'
keyword missing.
To solve this, make all SSAM_DEFINE_SYNC_REQUEST_x() macros define
static functions. Non-client-device macros are also changed for
consistency. In general, we expect those functions to be only used
locally in the respective drivers for the corresponding interfaces, so
having to define a wrapper function to be able to export this should be
the odd case out.
Reported-by: kernel test robot
Fixes: 510c8114fc74 ("platform/surface: Add platform profile driver")
Signed-off-by: Maximilian Luz
---
.../driver-api/surface_aggregator/client.rst | 4 +-
.../platform/surface/aggregator/controller.c | 10 +--
.../surface/surface_aggregator_registry.c | 2 +-
.../surface/surface_platform_profile.c| 4 +-
include/linux/surface_aggregator/controller.h | 74 +--
include/linux/surface_aggregator/device.h | 31
6 files changed, 63 insertions(+), 62 deletions(-)
diff --git a/Documentation/driver-api/surface_aggregator/client.rst
b/Documentation/driver-api/surface_aggregator/client.rst
index 26d13085a117..e519d374c378 100644
--- a/Documentation/driver-api/surface_aggregator/client.rst
+++ b/Documentation/driver-api/surface_aggregator/client.rst
@@ -248,7 +248,7 @@ This example defines a function
.. code-block:: c
- int __ssam_tmp_perf_mode_set(struct ssam_controller *ctrl, const __le32
*arg);
+ static int __ssam_tmp_perf_mode_set(struct ssam_controller *ctrl, const
__le32 *arg);
executing the specified request, with the controller passed in when calling
said function. In this example, the argument is provided via the ``arg``
@@ -296,7 +296,7 @@ This invocation of the macro defines a function
.. code-block:: c
- int ssam_bat_get_sta(struct ssam_device *sdev, __le32 *ret);
+ static int ssam_bat_get_sta(struct ssam_device *sdev, __le32 *ret);
executing the specified request, using the device IDs and controller given
in the client device. The full list of such macros for client devices is:
diff --git a/drivers/platform/surface/aggregator/controller.c
b/drivers/platform/surface/aggregator/controller.c
index 5bcb59ed579d..aa6f37b4f46e 100644
--- a/drivers/platform/surface/aggregator/controller.c
+++ b/drivers/platform/surface/aggregator/controller.c
@@ -1750,35 +1750,35 @@ EXPORT_SYMBOL_GPL(ssam_request_sync_with_buffer);
/* -- Internal SAM requests.
*/
-static SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_get_firmware_version, __le32, {
+SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_get_firmware_version, __le32, {
.target_category = SSAM_SSH_TC_SAM,
.target_id = 0x01,
.command_id = 0x13,
.instance_id = 0x00,
});
-static SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_display_off, u8, {
+SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_display_off, u8, {
.target_category = SSAM_SSH_TC_SAM,
.target_id = 0x01,
.command_id = 0x15,
.instance_id = 0x00,
});
-static SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_display_on, u8, {
+SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_display_on, u8, {
.target_category = SSAM_SSH_TC_SAM,
.target_id = 0x01,
.command_id = 0x16,
.instance_id = 0x00,
});
-static SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_d0_exit, u8, {
+SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_d0_exit, u8, {
.target_category = SSAM_SSH_TC_SAM,
.target_id = 0x01,
.command_id = 0x33,
.instance_id = 0x00,
});
-static SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_d0_entry, u8, {
+SSAM_DEFINE_SYNC_REQUEST_R(ssam_ssh_notif_d0_entry, u8, {
.target_category = SSAM_SSH_TC_SAM,
.target_id = 0x01,
.command_id = 0x34,
diff --git a/drivers/platform/surface/surface_aggregator_registry.c
b/drivers/platform/surface/surface_aggregator_registry.c
index
