Re: [PATCH v5 3/6] HID: himax: Add DRM panel follower support

[Krzysztof Kozlowski]

On Sat, Oct 03, 2026 at 04:27:38PM +0200, Michał Kopeć wrote:
> +/**
> + * himax_resume_proc() - Chip resume procedure of touch screen
> + * @ts: Himax touch screen data
> + *
> + * This function is used to resume the touch screen. It will call the
> + * himax_ap_notify_fw_suspend() to notify the FW of AP resume status.
> + *
> + * Return: None
> + */
> +static void himax_resume_proc(struct himax_ts_data *ts)
> +{
> +	himax_ap_notify_fw_suspend(ts, false);
> +}
> +
> +/**
> + * himax_chip_suspend() - Suspend the touch screen

Why are you explaining standard PM functions? Not only with comments,
but with kerneldoc?

> + * @ts: Himax touch screen data
> + *
> + * This function is used to suspend the touch screen. It will disable the
> + * interrupt and set the reset pin to activate state. Remove the HID at
> + * the end, to prevent stuck finger when resume.

Only the last sentence is relevant, all others just repeat the code. We
can read the code, no need to say what the code is doing. You should
explain WHY, not WHAT.

> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +static int himax_chip_suspend(struct himax_ts_data *ts)
> +{
> +	himax_int_enable(ts, false);
> +	gpiod_set_value(ts->pdata.gpiod_rst, 1);
> +	himax_power_set(ts, false);
> +	himax_hid_remove(ts);
> +
> +	return 0;
> +}

Please take over the series and clean it up from all these vendor kernel
monstrosities.

Best regards,
Krzysztof