linux-kernel - Re: [PATCH v2 5/6] gpio: max77759: add Maxim MAX77759 gpio driver

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: <CACRpkdYoWuJzjqiKrSNzdXV+5N9Gp0n+pdCwSZgocwy0JHo7Vw@mail.gmail.com>
Date: Fri, 28 Feb 2025 08:10:39 +0100
From: Linus Walleij <linus.walleij@...aro.org>
To: André Draszik <andre.draszik@...aro.org>
Cc: Lee Jones <lee@...nel.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Bartosz Golaszewski <brgl@...ev.pl>, Srinivas Kandagatla <srinivas.kandagatla@...aro.org>, 
	Kees Cook <kees@...nel.org>, "Gustavo A. R. Silva" <gustavoars@...nel.org>, 
	Peter Griffin <peter.griffin@...aro.org>, Tudor Ambarus <tudor.ambarus@...aro.org>, 
	Will McVicker <willmcvicker@...gle.com>, kernel-team@...roid.com, 
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-gpio@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2 5/6] gpio: max77759: add Maxim MAX77759 gpio driver

Hi André,

thanks for your patch!

mostly looks fine, given the MFD design is accepted.
Nitpicks below:

On Wed, Feb 26, 2025 at 6:51 PM André Draszik <andre.draszik@...aro.org> wrote:

> +static irqreturn_t max77759_gpio_irqhandler(int irq, void *data)
> +{
> +       int handled = 0;

bool handled = false;

(...)
> +               for_each_set_bit(offset, &pending, MAX77759_N_GPIOS) {
> +                       unsigned int virq;

I usually just call this "irq", as it's not any more virtual than any other
Linux magic number, and it can confuse people working with
actual virtualization when we call things virtual like this.

> +
> +                       /*
> +                        * ACK interrupt by writing 1 to bit 'offset', all
> +                        * others need to be written as 0. This needs to be
> +                        * done unconditionally hence regmap_set_bits() is
> +                        * inappropriate here.
> +                        */
> +                       regmap_write(chip->map, MAX77759_MAXQ_REG_UIC_INT1,
> +                                    BIT(offset));
> +
> +                       virq = irq_find_mapping(gc->irq.domain, offset);
> +                       handle_nested_irq(virq);
> +
> +                       handled = 1;

handled = true;

> +       chip->gc.base = -1;
> +       chip->gc.label = dev_name(&pdev->dev);
> +       chip->gc.parent = &pdev->dev;
> +       chip->gc.owner = THIS_MODULE;

The core already assigns THIS_MODULE so you can leave it
unassigned.

Yours,
Linus Walleij