Re: [PATCH 1/4] staging: nvec: make keyboard init synchronous

["Thierry Reding" <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2067 bytes --]

On Fri Apr 5, 2024 at 4:09 PM CEST, Marc Dietrich wrote:
> Improve initialization stability by waiting for command completion before
> sending the next one.
>
> Signed-off-by: Marc Dietrich <marvin24@gmx.de>
> ---
>  drivers/staging/nvec/nvec_kbd.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/nvec/nvec_kbd.c b/drivers/staging/nvec/nvec_kbd.c
> index f9a1da952c0a..6b203d28b8a9 100644
> --- a/drivers/staging/nvec/nvec_kbd.c
> +++ b/drivers/staging/nvec/nvec_kbd.c
> @@ -113,6 +113,7 @@ static int nvec_kbd_probe(struct platform_device *pdev)
>  		cnfg_wake[] = { NVEC_KBD, CNFG_WAKE, true, true },
>  		cnfg_wake_key_reporting[] = { NVEC_KBD, CNFG_WAKE_KEY_REPORTING,
>  						true };
> +	struct nvec_msg *msg;
>
>  	j = 0;
>
> @@ -148,15 +149,20 @@ static int nvec_kbd_probe(struct platform_device *pdev)
>  	nvec_register_notifier(nvec, &keys_dev.notifier, 0);
>
>  	/* Enable keyboard */
> -	nvec_write_async(nvec, enable_kbd, 2);
> +	nvec_write_sync(nvec, enable_kbd, 2, &msg);
> +	nvec_msg_free(nvec, msg);
>
>  	/* configures wake on special keys */
> -	nvec_write_async(nvec, cnfg_wake, 4);
> +	nvec_write_sync(nvec, cnfg_wake, 4, &msg);
> +	nvec_msg_free(nvec, msg);
> +
>  	/* enable wake key reporting */
> -	nvec_write_async(nvec, cnfg_wake_key_reporting, 3);
> +	nvec_write_sync(nvec, cnfg_wake_key_reporting, 3, &msg);
> +	nvec_msg_free(nvec, msg);
>
>  	/* Disable caps lock LED */
> -	nvec_write_async(nvec, clear_leds, sizeof(clear_leds));
> +	nvec_write_sync(nvec, clear_leds, sizeof(clear_leds), &msg);
> +	nvec_msg_free(nvec, msg);

I wonder if perhaps some of this duplication can be folded into
nvec_write_sync(). It seems a bit unnecessary to have to first get hold
of the last message only to immediately free it.

If nvec_write_sync() were allowed to take a NULL as msg parameter, then
we could check if this special case and simply do the nvec_msg_free()
from there directly. Not sure if it's really worth it, though.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]