linux-kernel - Re: [PATCH 1/4] gpio: tqmx86: add macros for interrupt configuration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMRc=MdDdkoMpd+pAJGTC1bMpNVb8D__5KGcxYK=OmOEP-C-Dw@mail.gmail.com>
Date: Wed, 11 Dec 2024 16:11:49 +0100
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Matthias Schiffer <matthias.schiffer@...tq-group.com>
Cc: Linus Walleij <linus.walleij@...aro.org>, linux@...tq-group.com, 
	linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] gpio: tqmx86: add macros for interrupt configuration

On Mon, Dec 9, 2024 at 11:36 AM Matthias Schiffer
<matthias.schiffer@...tq-group.com> wrote:
>
> We now consistently use TQMX86_INT_* flags for irq_type values. The
> TQMX86_GPII_CONFIG macro is used to convert from TQMX86_INT_TRIG_*
> flags to GPII register values. Bit patterns for TQMX86_INT_* are chosen
> to make this conversion as simple as possible.
>

Please use imperative clause in commit messages. Prefer "Consistently
use..." over "We now do this".

> No functional change intended.
>
> Signed-off-by: Matthias Schiffer <matthias.schiffer@...tq-group.com>
> ---
>  drivers/gpio/gpio-tqmx86.c | 43 ++++++++++++++++++++------------------
>  1 file changed, 23 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpio/gpio-tqmx86.c b/drivers/gpio/gpio-tqmx86.c
> index 5e26eb3adabbf..667cb34b882f0 100644
> --- a/drivers/gpio/gpio-tqmx86.c
> +++ b/drivers/gpio/gpio-tqmx86.c
> @@ -29,18 +29,21 @@
>  #define TQMX86_GPIIC   3       /* GPI Interrupt Configuration Register */
>  #define TQMX86_GPIIS   4       /* GPI Interrupt Status Register */
>
> -#define TQMX86_GPII_NONE       0
> -#define TQMX86_GPII_FALLING    BIT(0)
> -#define TQMX86_GPII_RISING     BIT(1)
> -/* Stored in irq_type as a trigger type, but not actually valid as a register
> - * value, so the name doesn't use "GPII"
> +/* NONE, FALLING and RISING use the same bit patterns that can be programmed to
> + * the GPII register (after passing them to the TQMX86_GPII_ macros to shift
> + * them to the right position)
>   */

If you're changing this, can you switch to using the preferred:

/*
 * foo
 */

pattern? Checkpatch should have warned you about this.

Looks good otherwise.

Bart