Re: [PATCH 11/11] HID: wacom: use 'hid_device_id::driver_data_ptr'

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] Incomplete refactoring of `id->driver_data` to `id->driver_data_ptr`.
- [Low] Mixed declarations and code in `wacom_probe()` and `wacom_wireless_work()`.
- [Low] Typo in the commit message.
--

commit 83b980c631eb908ccc27340ea394dd0fc86c568e
Author: Pawel Zalewski (The Capable Hub) <pzalewski@thegoodpenguin.co.uk>

HID: wacom: use 'hid_device_id::driver_data_ptr'

This commit updates the wacom driver to use the driver_data_ptr field of
struct hid_device_id instead of casting the driver_data field. It improves
code readability by avoiding explicit casts to kernel_ulong_t and
struct wacom_features *.

> Remove the casts to (kernel_ulong_t) and (struct wacom_features *)
> and use the 'drvier_data_ptr' field instead to improve readability
> of the code.

This isn't a bug, but there's a typo (drvier_data_ptr) in the commit message.

> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 0d1c6d90fe21c..25408676c5f61 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -2555,8 +2555,9 @@ static void wacom_wireless_work(struct work_struct *work)
>  		}
>  
>  		/* Stylus interface */
> -		wacom_wac1->features =
> -			*((struct wacom_features *)id->driver_data);
> +		const struct wacom_features *features = id->driver_data_ptr;
> +
> +		wacom_wac1->features = *features;

Does this introduce a mixed declaration and code warning? The variable
declaration is placed in the middle of a block after executable statements.

[ ... ]

> @@ -2839,7 +2839,11 @@ static int wacom_probe(struct hid_device *hdev,
>  	wacom->hdev = hdev;
>  
>  	wacom_wac = &wacom->wacom_wac;
> -	wacom_wac->features = *((struct wacom_features *)id->driver_data);
> +
> +	const struct wacom_features *id_features = id->driver_data_ptr;
> +
> +	wacom_wac->features = *id_features;

Similarly, does this placement of id_features violate the coding style by
mixing declarations and code?

Additionally, earlier in wacom_probe() there is this check:

	if (!id->driver_data)
		return -EINVAL;

Since the goal is to transition to driver_data_ptr, should that check also be
updated to use id->driver_data_ptr for consistency?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518-mod-devicetable-hid_device_id-v1-0-a08e3989c283@thegoodpenguin.co.uk?part=11