Skip to content

Commit

Permalink
platform/surface: aggregator: Rename top-level request functions to a…
Browse files Browse the repository at this point in the history
…void 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: sphinx-doc/sphinx#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>
Patchset: surface-sam
  • Loading branch information
qzed committed Sep 24, 2023
1 parent 99ac252 commit 7fefc5a
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 66 deletions.
8 changes: 4 additions & 4 deletions Documentation/driver-api/surface_aggregator/client.rst
Expand Up @@ -19,7 +19,7 @@
.. |ssam_notifier_unregister| replace:: :c:func:`ssam_notifier_unregister`
.. |ssam_device_notifier_register| replace:: :c:func:`ssam_device_notifier_register`
.. |ssam_device_notifier_unregister| replace:: :c:func:`ssam_device_notifier_unregister`
.. |ssam_request_sync| replace:: :c:func:`ssam_request_sync`
.. |ssam_request_do_sync| replace:: :c:func:`ssam_request_do_sync`
.. |ssam_event_mask| replace:: :c:type:`enum ssam_event_mask <ssam_event_mask>`


Expand Down Expand Up @@ -209,12 +209,12 @@ data received from it is converted from little-endian to host endianness.
* with the SSAM_REQUEST_HAS_RESPONSE flag set in the specification
* above.
*/
status = ssam_request_sync(ctrl, &rqst, &resp);
status = ssam_request_do_sync(ctrl, &rqst, &resp);
/*
* Alternatively use
*
* ssam_request_sync_onstack(ctrl, &rqst, &resp, sizeof(arg_le));
* ssam_request_do_sync_onstack(ctrl, &rqst, &resp, sizeof(arg_le));
*
* to perform the request, allocating the message buffer directly
* on the stack as opposed to allocation via kzalloc().
Expand All @@ -230,7 +230,7 @@ data received from it is converted from little-endian to host endianness.
return status;
}
Note that |ssam_request_sync| in its essence is a wrapper over lower-level
Note that |ssam_request_do_sync| in its essence is a wrapper over lower-level
request primitives, which may also be used to perform requests. Refer to its
implementation and documentation for more details.

Expand Down
6 changes: 3 additions & 3 deletions drivers/hid/surface-hid/surface_hid.c
Expand Up @@ -80,7 +80,7 @@ static int ssam_hid_get_descriptor(struct surface_hid_device *shid, u8 entry, u8

rsp.length = 0;

status = ssam_retry(ssam_request_sync_onstack, shid->ctrl, &rqst, &rsp,
status = ssam_retry(ssam_request_do_sync_onstack, shid->ctrl, &rqst, &rsp,
sizeof(*slice));
if (status)
return status;
Expand Down Expand Up @@ -131,7 +131,7 @@ static int ssam_hid_set_raw_report(struct surface_hid_device *shid, u8 rprt_id,

buf[0] = rprt_id;

return ssam_retry(ssam_request_sync, shid->ctrl, &rqst, NULL);
return ssam_retry(ssam_request_do_sync, shid->ctrl, &rqst, NULL);
}

static int ssam_hid_get_raw_report(struct surface_hid_device *shid, u8 rprt_id, u8 *buf, size_t len)
Expand All @@ -151,7 +151,7 @@ static int ssam_hid_get_raw_report(struct surface_hid_device *shid, u8 rprt_id,
rsp.length = 0;
rsp.pointer = buf;

return ssam_retry(ssam_request_sync_onstack, shid->ctrl, &rqst, &rsp, sizeof(rprt_id));
return ssam_retry(ssam_request_do_sync_onstack, shid->ctrl, &rqst, &rsp, sizeof(rprt_id));
}

static u32 ssam_hid_event_fn(struct ssam_event_notifier *nf, const struct ssam_event *event)
Expand Down
6 changes: 3 additions & 3 deletions drivers/hid/surface-hid/surface_kbd.c
Expand Up @@ -49,7 +49,7 @@ static int ssam_kbd_get_descriptor(struct surface_hid_device *shid, u8 entry, u8
rsp.length = 0;
rsp.pointer = buf;

status = ssam_retry(ssam_request_sync_onstack, shid->ctrl, &rqst, &rsp, sizeof(entry));
status = ssam_retry(ssam_request_do_sync_onstack, shid->ctrl, &rqst, &rsp, sizeof(entry));
if (status)
return status;

Expand All @@ -75,7 +75,7 @@ static int ssam_kbd_set_caps_led(struct surface_hid_device *shid, bool value)
rqst.length = sizeof(value_u8);
rqst.payload = &value_u8;

return ssam_retry(ssam_request_sync_onstack, shid->ctrl, &rqst, NULL, sizeof(value_u8));
return ssam_retry(ssam_request_do_sync_onstack, shid->ctrl, &rqst, NULL, sizeof(value_u8));
}

static int ssam_kbd_get_feature_report(struct surface_hid_device *shid, u8 *buf, size_t len)
Expand All @@ -97,7 +97,7 @@ static int ssam_kbd_get_feature_report(struct surface_hid_device *shid, u8 *buf,
rsp.length = 0;
rsp.pointer = buf;

status = ssam_retry(ssam_request_sync_onstack, shid->ctrl, &rqst, &rsp, sizeof(payload));
status = ssam_retry(ssam_request_do_sync_onstack, shid->ctrl, &rqst, &rsp, sizeof(payload));
if (status)
return status;

Expand Down
6 changes: 3 additions & 3 deletions drivers/platform/surface/aggregator/bus.c
Expand Up @@ -136,9 +136,9 @@ int ssam_device_add(struct ssam_device *sdev)
* is always valid and can be used for requests as long as the client
* device we add here is registered as child under it. This essentially
* guarantees that the client driver can always expect the preconditions
* for functions like ssam_request_sync (controller has to be started
* and is not suspended) to hold and thus does not have to check for
* them.
* for functions like ssam_request_do_sync() (controller has to be
* started and is not suspended) to hold and thus does not have to check
* for them.
*
* Note that for this to work, the controller has to be a parent device.
* If it is not a direct parent, care has to be taken that the device is
Expand Down
32 changes: 16 additions & 16 deletions drivers/platform/surface/aggregator/controller.c
Expand Up @@ -1674,7 +1674,7 @@ int ssam_request_sync_submit(struct ssam_controller *ctrl,
EXPORT_SYMBOL_GPL(ssam_request_sync_submit);

/**
* ssam_request_sync() - Execute a synchronous request.
* ssam_request_do_sync() - Execute a synchronous request.
* @ctrl: The controller via which the request will be submitted.
* @spec: The request specification and payload.
* @rsp: The response buffer.
Expand All @@ -1686,9 +1686,9 @@ EXPORT_SYMBOL_GPL(ssam_request_sync_submit);
*
* Return: Returns the status of the request or any failure during setup.
*/
int ssam_request_sync(struct ssam_controller *ctrl,
const struct ssam_request *spec,
struct ssam_response *rsp)
int ssam_request_do_sync(struct ssam_controller *ctrl,
const struct ssam_request *spec,
struct ssam_response *rsp)
{
struct ssam_request_sync *rqst;
struct ssam_span buf;
Expand Down Expand Up @@ -1722,10 +1722,10 @@ int ssam_request_sync(struct ssam_controller *ctrl,
ssam_request_sync_free(rqst);
return status;
}
EXPORT_SYMBOL_GPL(ssam_request_sync);
EXPORT_SYMBOL_GPL(ssam_request_do_sync);

/**
* ssam_request_sync_with_buffer() - Execute a synchronous request with the
* ssam_request_do_sync_with_buffer() - Execute a synchronous request with the
* provided buffer as back-end for the message buffer.
* @ctrl: The controller via which the request will be submitted.
* @spec: The request specification and payload.
Expand All @@ -1738,17 +1738,17 @@ EXPORT_SYMBOL_GPL(ssam_request_sync);
* SSH_COMMAND_MESSAGE_LENGTH() macro can be used to compute the required
* message buffer size.
*
* This function does essentially the same as ssam_request_sync(), but instead
* of dynamically allocating the request and message data buffer, it uses the
* provided message data buffer and stores the (small) request struct on the
* heap.
* This function does essentially the same as ssam_request_do_sync(), but
* instead of dynamically allocating the request and message data buffer, it
* uses the provided message data buffer and stores the (small) request struct
* on the heap.
*
* Return: Returns the status of the request or any failure during setup.
*/
int ssam_request_sync_with_buffer(struct ssam_controller *ctrl,
const struct ssam_request *spec,
struct ssam_response *rsp,
struct ssam_span *buf)
int ssam_request_do_sync_with_buffer(struct ssam_controller *ctrl,
const struct ssam_request *spec,
struct ssam_response *rsp,
struct ssam_span *buf)
{
struct ssam_request_sync rqst;
ssize_t len;
Expand All @@ -1772,7 +1772,7 @@ int ssam_request_sync_with_buffer(struct ssam_controller *ctrl,

return status;
}
EXPORT_SYMBOL_GPL(ssam_request_sync_with_buffer);
EXPORT_SYMBOL_GPL(ssam_request_do_sync_with_buffer);


/* -- Internal SAM requests. ------------------------------------------------ */
Expand Down Expand Up @@ -1864,7 +1864,7 @@ static int __ssam_ssh_event_request(struct ssam_controller *ctrl,
result.length = 0;
result.pointer = &buf;

status = ssam_retry(ssam_request_sync_onstack, ctrl, &rqst, &result,
status = ssam_retry(ssam_request_do_sync_onstack, ctrl, &rqst, &result,
sizeof(params));

return status < 0 ? status : buf;
Expand Down
2 changes: 1 addition & 1 deletion drivers/platform/surface/surface_acpi_notify.c
Expand Up @@ -590,7 +590,7 @@ static acpi_status san_rqst(struct san_data *d, struct gsb_buffer *buffer)
return san_rqst_fixup_suspended(d, &rqst, buffer);
}

status = __ssam_retry(ssam_request_sync_onstack, SAN_REQUEST_NUM_TRIES,
status = __ssam_retry(ssam_request_do_sync_onstack, SAN_REQUEST_NUM_TRIES,
d->ctrl, &rqst, &rsp, SAN_GSB_MAX_RQSX_PAYLOAD);

if (!status) {
Expand Down
6 changes: 3 additions & 3 deletions drivers/platform/surface/surface_aggregator_cdev.c
Expand Up @@ -302,8 +302,8 @@ static long ssam_cdev_request(struct ssam_cdev_client *client, struct ssam_cdev_
* theoretical maximum (SSH_COMMAND_MAX_PAYLOAD_SIZE) of the
* underlying protocol (note that nothing remotely this size
* should ever be allocated in any normal case). This size is
* validated later in ssam_request_sync(), for allocation the
* bound imposed by u16 should be enough.
* validated later in ssam_request_do_sync(), for allocation
* the bound imposed by u16 should be enough.
*/
spec.payload = kzalloc(spec.length, GFP_KERNEL);
if (!spec.payload) {
Expand Down Expand Up @@ -342,7 +342,7 @@ static long ssam_cdev_request(struct ssam_cdev_client *client, struct ssam_cdev_
}

/* Perform request. */
status = ssam_request_sync(client->cdev->ctrl, &spec, &rsp);
status = ssam_request_do_sync(client->cdev->ctrl, &spec, &rsp);
if (status)
goto out;

Expand Down
2 changes: 1 addition & 1 deletion drivers/platform/surface/surface_aggregator_tabletsw.c
Expand Up @@ -387,7 +387,7 @@ static int ssam_pos_get_sources_list(struct ssam_tablet_sw *sw, struct ssam_sour
rsp.length = 0;
rsp.pointer = (u8 *)sources;

status = ssam_retry(ssam_request_sync_onstack, sw->sdev->ctrl, &rqst, &rsp, 0);
status = ssam_retry(ssam_request_do_sync_onstack, sw->sdev->ctrl, &rqst, &rsp, 0);
if (status)
return status;

Expand Down

0 comments on commit 7fefc5a

Please sign in to comment.