| Message ID | 20250430182300.122896-1-chelsyratnawat2001@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | HID: sensor-hub: Fix typo and improve documentation for sensor_hub_remove_callback() | expand |
On Wed, 2025-04-30 at 14:17 -0500, David Lechner wrote: > On 4/30/25 1:23 PM, Chelsy Ratnawat wrote: > > Fixed a typo in "registered" and improved grammar for better > > readability > > and consistency with kernel-doc standards. No functional changes. > > > > Signed-off-by: Chelsy Ratnawat <chelsyratnawat2001@gmail.com> > > --- > > include/linux/hid-sensor-hub.h | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid- > > sensor-hub.h > > index c27329e2a5ad..5d2ac79429d4 100644 > > --- a/include/linux/hid-sensor-hub.h > > +++ b/include/linux/hid-sensor-hub.h > > @@ -130,10 +130,11 @@ int sensor_hub_register_callback(struct > > hid_sensor_hub_device *hsdev, > > /** > > * sensor_hub_remove_callback() - Remove client callbacks > > This says "callbacks", so is it possible to have more than one > registered at a > time? This removes only one. So "callback" will be correct. Thanks, Srinivas > > > * @hsdev: Hub device instance. > > -* @usage_id: Usage id of the client (E.g. 0x200076 for Gyro). > > +* @usage_id: Usage id of the client (e.g. 0x200076 for Gyro). > > should we also make gyro lower-case? > > > * > > -* If there is a callback registred, this call will remove that > > -* callbacks, so that it will stop data and event notifications. > > +* Removes a previously registered callback for the given usage ID. > > +* Once removed, the client will no longer receive data or event > > +* notifications. > > I like the revised wording, but possibly looses some clarity that > could be > fixed with: > > Removes a previously registered callback(s), if any, for the given > usage ID. > > As above, not sure if singular or plural callbacks is correct. > > > */ > > int sensor_hub_remove_callback(struct hid_sensor_hub_device > > *hsdev, > > u32 usage_id); >
On 5/1/25 6:19 PM, Chelsy Ratnawat wrote: > Changes in v2: > - Improved the kernel-doc comment for sensor_hub_remove_callback(). > - Changed "Gyro" to "gyro". > - Changed "usage ID" to "usage_id" for consistency with kernel-doc > style. > - Updated the comment to state that only one callback can be removed > per (usage_id, hsdev) pair. > > Signed-off-by: Chelsy Ratnawat <chelsyratnawat2001@gmail.com> > --- Normally people put the changes here below the --- rather than putting it in the commit message. Patch part looks good though. Reviewed-by: David Lechner <dlechner@baylibre.com>
On 5/1/25 6:33 PM, chelsy ratnawat wrote: > Hi, Watch out for HTML mail! The mailing list and other automated tools will reject it, so some people won't see the whole conversation. > Thanks for the feedback. Regarding your comments: > > On Thu, May 1, 2025 at 12:47 AM David Lechner <dlechner@baylibre.com <mailto:dlechner@baylibre.com>> wrote: > > On 4/30/25 1:23 PM, Chelsy Ratnawat wrote: > > Fixed a typo in "registered" and improved grammar for better readability > > and consistency with kernel-doc standards. No functional changes. > > > > Signed-off-by: Chelsy Ratnawat <chelsyratnawat2001@gmail.com <mailto:chelsyratnawat2001@gmail.com>> > > --- > > include/linux/hid-sensor-hub.h | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h > > index c27329e2a5ad..5d2ac79429d4 100644 > > --- a/include/linux/hid-sensor-hub.h > > +++ b/include/linux/hid-sensor-hub.h > > @@ -130,10 +130,11 @@ int sensor_hub_register_callback(struct hid_sensor_hub_device *hsdev, > > /** > > * sensor_hub_remove_callback() - Remove client callbacks > > This says "callbacks", so is it possible to have more than one registered at a > time? > > > Regarding the use of "callback" instead of "callbacks", what I understand is- > - The function `sensor_hub_register_callback()` ensures that only one callback is registered for each `(hsdev, usage_id)` pair. If another callback is registered for the same `(hsdev, usage_id)`, it returns `-EINVAL`. > - Therefore, `sensor_hub_remove_callback()` is designed to remove that single registered callback for a given `(hsdev, usage_id)` pair. The function does not need to handle multiple callbacks for the same pair, as only one > callback is registered at a time. > > Please let me know if my understanding is correct, or if you have any additional feedback or suggestions. Based on the reply from Srinivas, it sounds like you understand correctly.
diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h index c27329e2a5ad..5d2ac79429d4 100644 --- a/include/linux/hid-sensor-hub.h +++ b/include/linux/hid-sensor-hub.h @@ -130,10 +130,11 @@ int sensor_hub_register_callback(struct hid_sensor_hub_device *hsdev, /** * sensor_hub_remove_callback() - Remove client callbacks * @hsdev: Hub device instance. -* @usage_id: Usage id of the client (E.g. 0x200076 for Gyro). +* @usage_id: Usage id of the client (e.g. 0x200076 for Gyro). * -* If there is a callback registred, this call will remove that -* callbacks, so that it will stop data and event notifications. +* Removes a previously registered callback for the given usage ID. +* Once removed, the client will no longer receive data or event +* notifications. */ int sensor_hub_remove_callback(struct hid_sensor_hub_device *hsdev, u32 usage_id);
Fixed a typo in "registered" and improved grammar for better readability and consistency with kernel-doc standards. No functional changes. Signed-off-by: Chelsy Ratnawat <chelsyratnawat2001@gmail.com> --- include/linux/hid-sensor-hub.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)