Re: [PATCH v5 03/13] HID: hid-sensor-hub: introduce device managed API

On Thu, Aug 06, 2026 at 05:55:49PM +0530, Sanjay Chitroda via B4 Relay wrote:

> sensor_hub_register_callback() is common API used for the HID IIO drivers,
> prepare devm API devm_sensor_hub_register_callback() to acquire resource
> during setup and release using device managed framework during drivers
> fail, unbind or remove path.
> 
> store the required callback removal context (hsdev and usage_id) in a

Store

> dedicated struct sensor_hub_cb_devres, and register a devres action to
> handle to release resource with devres framework and helper API.

...

> +static void sensor_hub_remove_callback_helper(void *ptr)
> +{
> +	struct sensor_hub_cb_devres *res = ptr;
> +
> +	sensor_hub_remove_callback(res->hsdev, res->usage_id);
> +}
> +
> +int devm_sensor_hub_register_callback(struct device *dev,
> +				      struct hid_sensor_hub_device *hsdev,
> +				      u32 usage_id,
> +				      struct hid_sensor_hub_callbacks *usage_callback)
> +{
> +	struct sensor_hub_cb_devres *res;
> +	int ret;
> +
> +	ret = sensor_hub_register_callback(hsdev, usage_id, usage_callback);
> +	if (ret)
> +		return ret;
> +
> +	res = devm_kmalloc(dev, sizeof(*res), GFP_KERNEL);
> +	if (!res) {
> +		sensor_hub_remove_callback(hsdev, usage_id);
> +		return -ENOMEM;
> +	}

Why is this order? What's wrong with the memory allocation first?

> +	res->hsdev = hsdev;
> +	res->usage_id = usage_id;
> +
> +	return devm_add_action_or_reset(dev, sensor_hub_remove_callback_helper, res);
> +}

...

> +/**
> + * devm_sensor_hub_register_callback() - Managed register client callbacks
> + * @dev: Device for resource management
> + * @hsdev: Hub device instance
> + * @usage_id: Usage id of the client (e.g. 0x200076 for Gyro)
> + * @usage_callback: Callback function storage
> + *
> + * This is the devres (managed) version of sensor_hub_register_callback().
> + * The callback will be automatically unregistered when the device is detached.

Here, and everywhere else, validate kernel-doc. Here you introduced a new warning
(missing return section).

> + */

Also, why kernel-doc is here?! It should accompany the implementation, here is
a declaration.

> +int devm_sensor_hub_register_callback(struct device *dev,
> +				      struct hid_sensor_hub_device *hsdev,
> +				      u32 usage_id,
> +				      struct hid_sensor_hub_callbacks *usage_callback);

-- 
With Best Regards,
Andy Shevchenko