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

[Linus Walleij]

On Tue, Feb 25, 2020 at 5:42 PM Asmaa Mnebhi <Asmaa@xxxxxxxxxxxx> 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