From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Glass <sjg@chromium.org>
Date: Mon, 4 May 2020 08:17:55 -0600
Subject: [PATCH v4 1/2] drivers: gpio: add broadcom iproc gpio driver
 support
In-Reply-To: <20200503163153.11532-2-rayagonda.kokatanur@broadcom.com>
References: <20200503163153.11532-1-rayagonda.kokatanur@broadcom.com>
 <20200503163153.11532-2-rayagonda.kokatanur@broadcom.com>
Message-ID: <CAPnjgZ1bdtEFD5YebCdWGLEdPUNkOKT3mud8rxqieDakLBatEQ@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Rayagonda,

On Sun, 3 May 2020 at 10:32, Rayagonda Kokatanur
<rayagonda.kokatanur@broadcom.com> wrote:
>
> Add gpio driver support for Broadcom iproc-based socs.
>
> Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> Signed-off-by: Sheetal Tigadoli <sheetal.tigadoli@broadcom.com>
> ---
>  drivers/gpio/Kconfig      |  11 ++
>  drivers/gpio/Makefile     |   1 +
>  drivers/gpio/iproc_gpio.c | 269 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 281 insertions(+)
>  create mode 100644 drivers/gpio/iproc_gpio.c
>
[..]

> +static int iproc_gpio_ofdata_to_platdata(struct udevice *dev)
> +{
> +       struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
> +       struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> +       int ret;
> +       char name[10];
> +
> +       plat->base = dev_read_addr_ptr(dev);
> +       if (!plat->base) {
> +               debug("%s: Failed to get base address\n", __func__);
> +               return -EINVAL;
> +       }
> +
> +       ret = dev_read_u32(dev, "ngpios", &plat->ngpios);
> +       if (ret < 0) {
> +               dev_err(dev, "%s: Failed to get ngpios\n", __func__);
> +               return ret;
> +       }
> +
> +       uclass_get_device_by_phandle(UCLASS_PINCTRL, dev, "gpio-ranges",
> +                                    &plat->pinctrl_dev);

Should check return value here.

> +
> +       INIT_LIST_HEAD(&plat->gpiomap);
> +       ret = iproc_get_gpio_pctrl_mapping(dev);

This is confusing, please drop the !

> +       if (ret) {
> +               dev_err(dev, "%s: Failed to get gpio to pctrl map ret(%d)\n",
> +                       __func__, ret);
> +               return ret;
> +       }
> +
> +       snprintf(name, sizeof(name), "GPIO%d", dev->req_seq);
> +       plat->name = strdup(name);

Check for error and return -ENOMEM

> +
> +       uc_priv->gpio_count = plat->ngpios;
> +       uc_priv->bank_name = plat->name;
> +
> +       dev_info(dev, ":bank name(%s) base %p, #gpios %d\n",
> +                plat->name, plat->base, plat->ngpios);
> +
> +       return 0;
> +}
> +

[..]


Regards,
Simon