linux-kernel - Re: [PATCH v2 1/1] gpio: add driver for Mellanox BlueField 2 GPIO controller

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] [day] [month] [year] [list]

Message-ID: <CACRpkdZhs=ZYkcYb5bNqK_ayWEVk9=J0w--ELW-vcvoSvG9cxg@mail.gmail.com>
Date:   Sat, 29 Feb 2020 00:14:00 +0100
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Asmaa Mnebhi <Asmaa@...lanox.com>
Cc:     "bgolaszewski@...libre.com" <bgolaszewski@...libre.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/1] gpio: add driver for Mellanox BlueField 2 GPIO controller

On Tue, Feb 25, 2020 at 5:42 PM Asmaa Mnebhi <Asmaa@...lanox.com> wrote:

> I have addressed your comments and tested the code. Please note
>  that the YU_ARM_GPIO_LOCK is a shared resource between the 3
>  gpio blocks instances.
> So now that we have split up the gpio_chip into 3 instances, we need to share
>  that LOCK resource accordingly. I have created a global variable for that purpose.

Fair enough, all looks so much nicer now :)

This:

+       /*
+        * Although the arm_gpio_lock was set in the probe function,
+        * check again it is still enabled to be able to write to the
+        * ModeX registers.
+        */
+       spin_lock(&gs->gc.bgpio_lock);
+       ret = mlxbf2_gpio_lock_acquire();
+       if (ret < 0) {
+               spin_unlock(&gs->gc.bgpio_lock);
+               return ret;
+       }

Is open-coded in two places, please create a helper function for this
or just merge it into mlbx2_gpio_lock_acquire() since it is done all the
time when that is called.

> Also note, that although it is not supported in this driver at the moment, some
> gpio interrupt registers will be similar to that LOCK i.e. they are shared among
> the  3 gpio block instances.

It's a good start like this.
The robot complains about devm_ioremap_nocache
which is now deleted, use devm_ioremap() simply, they are
all write-through.

Yours,
Linus Walleij