* [PATCH v5 0/2] add brcm iproc gpio driver
@ 2020-05-04 15:59 Rayagonda Kokatanur
2020-05-04 15:59 ` [PATCH v5 1/2] drivers: gpio: add broadcom iproc gpio driver support Rayagonda Kokatanur
2020-05-04 15:59 ` [PATCH v5 2/2] gpio: do not include <asm/arch/gpio.h> on TARGET_BCMNS3 Rayagonda Kokatanur
0 siblings, 2 replies; 8+ messages in thread
From: Rayagonda Kokatanur @ 2020-05-04 15:59 UTC (permalink / raw)
To: u-boot
Add support for broadcom iproc gpio driver.
Changes from v4:
-Address review comments from Simon Glass,
Check for function return value.
Changes from v3:
-Address review comments from Simon Glass,
Rearrange header file,
Add struct comment taking about members,
Use clrsetbits_le32(),
Add new line before return etc.
Changes from v2:
-remove ununsed variable "node" in iproc_get_gpio_pctrl_mapping()
func to fix compilation warning.
Changes from v1:
-Address review comments from Simon Glass,
Use dev_read_...() instead of fdtdec_...().
Rayagonda Kokatanur (2):
drivers: gpio: add broadcom iproc gpio driver support
gpio: do not include <asm/arch/gpio.h> on TARGET_BCMNS3
arch/arm/include/asm/gpio.h | 2 +-
drivers/gpio/Kconfig | 11 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/iproc_gpio.c | 275 ++++++++++++++++++++++++++++++++++++
4 files changed, 288 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpio/iproc_gpio.c
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v5 1/2] drivers: gpio: add broadcom iproc gpio driver support
2020-05-04 15:59 [PATCH v5 0/2] add brcm iproc gpio driver Rayagonda Kokatanur
@ 2020-05-04 15:59 ` Rayagonda Kokatanur
2020-05-04 19:26 ` Simon Glass
2020-05-05 16:02 ` Simon Glass
2020-05-04 15:59 ` [PATCH v5 2/2] gpio: do not include <asm/arch/gpio.h> on TARGET_BCMNS3 Rayagonda Kokatanur
1 sibling, 2 replies; 8+ messages in thread
From: Rayagonda Kokatanur @ 2020-05-04 15:59 UTC (permalink / raw)
To: u-boot
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 | 275 ++++++++++++++++++++++++++++++++++++++
3 files changed, 287 insertions(+)
create mode 100644 drivers/gpio/iproc_gpio.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 2081520f42..57d2cd2e75 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -135,6 +135,17 @@ config IMX_RGPIO2P
help
This driver supports i.MX7ULP Rapid GPIO2P controller.
+config IPROC_GPIO
+ bool "Broadcom iProc GPIO driver(without pinconf)"
+ default n
+ help
+ The Broadcom iProc based SoCs- Cygnus, NS2, NS3, NSP and Stingray,
+ use the same GPIO Controller IP hence this driver could be used
+ for all.
+
+ The Broadcom iProc based SoCs have multiple GPIO controllers and only
+ the always-ON GPIO controller (CRMU/AON) is supported by this driver.
+
config HSDK_CREG_GPIO
bool "HSDK CREG GPIO griver"
depends on DM_GPIO
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 7638259007..5dc5849477 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_CORTINA_GPIO) += cortina_gpio.o
obj-$(CONFIG_INTEL_GPIO) += intel_gpio.o
obj-$(CONFIG_INTEL_ICH6_GPIO) += intel_ich6_gpio.o
obj-$(CONFIG_INTEL_BROADWELL_GPIO) += intel_broadwell_gpio.o
+obj-$(CONFIG_IPROC_GPIO) += iproc_gpio.o
obj-$(CONFIG_KIRKWOOD_GPIO) += kw_gpio.o
obj-$(CONFIG_KONA_GPIO) += kona_gpio.o
obj-$(CONFIG_MARVELL_GPIO) += mvgpio.o
diff --git a/drivers/gpio/iproc_gpio.c b/drivers/gpio/iproc_gpio.c
new file mode 100644
index 0000000000..f75831d6e7
--- /dev/null
+++ b/drivers/gpio/iproc_gpio.c
@@ -0,0 +1,275 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2020 Broadcom
+ */
+
+#include <common.h>
+#include <errno.h>
+#include <asm/gpio.h>
+#include <asm/io.h>
+#include <dm.h>
+#include <dm/device_compat.h>
+#include <dm/devres.h>
+#include <dm/pinctrl.h>
+
+/*
+ * There are five GPIO bank register. Each bank can configure max of 32 gpios.
+ * BANK0 - gpios 0 to 31
+ * BANK1 - gpios 32 to 63
+ * BANK2 - gpios 64 to 95
+ * BANK3 - gpios 96 to 127
+ * BANK4 - gpios 128 to 150
+ *
+ * Offset difference between consecutive bank register is 0x200
+ */
+#define NGPIO_PER_BANK 32
+#define GPIO_BANK_SIZE 0x200
+#define GPIO_BANK(pin) ((pin) / NGPIO_PER_BANK)
+#define IPROC_GPIO_SHIFT(pin) ((pin) % NGPIO_PER_BANK)
+#define IPROC_GPIO_REG(pin, reg) (GPIO_BANK_SIZE * GPIO_BANK(pin) + (reg))
+
+/* device register offset */
+#define IPROC_GPIO_DATA_IN_OFFSET 0x00
+#define IPROC_GPIO_DATA_OUT_OFFSET 0x04
+#define IPROC_GPIO_OUT_EN_OFFSET 0x08
+
+/**
+ * struct iproc_gpio_pctrl_map - mapping between gpio and pinctrl specfied
+ * using gpio-ranges parameter in dt.
+ * @gpio_pin: start of gpio number in gpio-ranges
+ * @pctrl_pin: start of pinctrl number in gpio-ranges
+ * @npins: total number of pins in gpio-ranges
+ * @node: list node
+ */
+struct iproc_gpio_pctrl_map {
+ u32 gpio_pin;
+ u32 pctrl_pin;
+ u32 npins;
+ struct list_head node;
+};
+
+/**
+ * struct iproc_gpio_pctrl_map - gpio device instance
+ * @pinctrl_dev:pointer to pinctrl device
+ * @gpiomap: list node having mapping between gpio and pinctrl
+ * @base: I/O register base address of gpio device
+ * @name: gpio device name, ex GPIO0, GPIO1
+ * @ngpios: total number of gpios
+ */
+struct iproc_gpio_platdata {
+ struct udevice *pinctrl_dev;
+ struct list_head gpiomap;
+ void __iomem *base;
+ char *name;
+ u32 ngpios;
+};
+
+/**
+ * iproc_gpio_set_bit - set or clear one bit (corresponding to the GPIO pin)
+ * in a iproc GPIO register
+ *
+ * @iproc_gpio: Iproc GPIO device
+ * @reg: register offset
+ * @gpio: GPIO pin
+ * @set: set or clear
+ */
+static inline void iproc_gpio_set_bit(struct iproc_gpio_platdata *plat,
+ u32 reg, u32 gpio, bool set)
+{
+ u32 offset = IPROC_GPIO_REG(gpio, reg);
+ u32 shift = IPROC_GPIO_SHIFT(gpio);
+
+ clrsetbits_le32(plat->base + offset, BIT(shift),
+ (set ? BIT(shift) : 0));
+}
+
+static inline bool iproc_gpio_get_bit(struct iproc_gpio_platdata *plat,
+ u32 reg, u32 gpio)
+{
+ u32 offset = IPROC_GPIO_REG(gpio, reg);
+ u32 shift = IPROC_GPIO_SHIFT(gpio);
+
+ return readl(plat->base + offset) & BIT(shift);
+}
+
+static u32 iproc_get_pctrl_from_gpio(struct iproc_gpio_platdata *plat, u32 gpio)
+{
+ struct iproc_gpio_pctrl_map *range = NULL;
+ struct list_head *pos, *tmp;
+ u32 ret = 0;
+
+ list_for_each_safe(pos, tmp, &plat->gpiomap) {
+ range = list_entry(pos, struct iproc_gpio_pctrl_map, node);
+ if (gpio == range->gpio_pin ||
+ gpio < (range->gpio_pin + range->npins)) {
+ ret = range->pctrl_pin + (gpio - range->gpio_pin);
+ break;
+ }
+ }
+
+ return ret;
+}
+
+static int iproc_get_gpio_pctrl_mapping(struct udevice *dev)
+{
+ struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
+ struct iproc_gpio_pctrl_map *range = NULL;
+ struct ofnode_phandle_args args;
+ int index = 0, ret;
+
+ for (;; index++) {
+ ret = dev_read_phandle_with_args(dev, "gpio-ranges",
+ NULL, 3, index, &args);
+ if (ret)
+ break;
+
+ range = devm_kzalloc(dev, sizeof(*range), GFP_KERNEL);
+ if (!range)
+ return -ENOMEM;
+
+ range->gpio_pin = args.args[0];
+ range->pctrl_pin = args.args[1];
+ range->npins = args.args[2];
+ list_add_tail(&range->node, &plat->gpiomap);
+ }
+
+ return 0;
+}
+
+static int iproc_gpio_request(struct udevice *dev, u32 gpio, const char *label)
+{
+ struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
+ u32 pctrl;
+
+ if (!plat->pinctrl_dev)
+ return 0;
+
+ pctrl = iproc_get_pctrl_from_gpio(plat, gpio);
+
+ return pinctrl_request(plat->pinctrl_dev, pctrl, 0);
+}
+
+static int iproc_gpio_direction_input(struct udevice *dev, u32 gpio)
+{
+ struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
+
+ iproc_gpio_set_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio, false);
+ dev_dbg(dev, "gpio:%u set input\n", gpio);
+
+ return 0;
+}
+
+static int iproc_gpio_direction_output(struct udevice *dev, u32 gpio, int value)
+{
+ struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
+
+ iproc_gpio_set_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio, true);
+ iproc_gpio_set_bit(plat, IPROC_GPIO_DATA_OUT_OFFSET, gpio, value);
+ dev_dbg(dev, "gpio:%u set output, value:%d\n", gpio, value);
+
+ return 0;
+}
+
+static int iproc_gpio_get_value(struct udevice *dev, u32 gpio)
+{
+ struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
+ int value;
+
+ value = iproc_gpio_get_bit(plat, IPROC_GPIO_DATA_IN_OFFSET, gpio);
+ dev_dbg(dev, "gpio:%u get, value:%d\n", gpio, value);
+
+ return value;
+}
+
+static int iproc_gpio_set_value(struct udevice *dev, u32 gpio, int value)
+{
+ struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
+
+ if (iproc_gpio_get_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio))
+ iproc_gpio_set_bit(plat, IPROC_GPIO_DATA_OUT_OFFSET, gpio,
+ value);
+
+ dev_dbg(dev, "gpio:%u set, value:%d\n", gpio, value);
+ return 0;
+}
+
+static int iproc_gpio_get_function(struct udevice *dev, u32 gpio)
+{
+ struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
+
+ if (iproc_gpio_get_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio))
+ return GPIOF_OUTPUT;
+ else
+ return GPIOF_INPUT;
+}
+
+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);
+ if (ret < 0) {
+ dev_err(dev, "%s: Failed to get pinctrl phandle\n", __func__);
+ return ret;
+ }
+
+ INIT_LIST_HEAD(&plat->gpiomap);
+ ret = iproc_get_gpio_pctrl_mapping(dev);
+ if (ret < 0) {
+ 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);
+ if (!plat->name)
+ 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;
+}
+
+static const struct dm_gpio_ops iproc_gpio_ops = {
+ .request = iproc_gpio_request,
+ .direction_input = iproc_gpio_direction_input,
+ .direction_output = iproc_gpio_direction_output,
+ .get_value = iproc_gpio_get_value,
+ .set_value = iproc_gpio_set_value,
+ .get_function = iproc_gpio_get_function,
+};
+
+static const struct udevice_id iproc_gpio_ids[] = {
+ { .compatible = "brcm,iproc-gpio" },
+ { }
+};
+
+U_BOOT_DRIVER(iproc_gpio) = {
+ .name = "iproc_gpio",
+ .id = UCLASS_GPIO,
+ .of_match = iproc_gpio_ids,
+ .ops = &iproc_gpio_ops,
+ .ofdata_to_platdata = iproc_gpio_ofdata_to_platdata,
+ .platdata_auto_alloc_size = sizeof(struct iproc_gpio_platdata),
+};
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v5 2/2] gpio: do not include <asm/arch/gpio.h> on TARGET_BCMNS3
2020-05-04 15:59 [PATCH v5 0/2] add brcm iproc gpio driver Rayagonda Kokatanur
2020-05-04 15:59 ` [PATCH v5 1/2] drivers: gpio: add broadcom iproc gpio driver support Rayagonda Kokatanur
@ 2020-05-04 15:59 ` Rayagonda Kokatanur
1 sibling, 0 replies; 8+ messages in thread
From: Rayagonda Kokatanur @ 2020-05-04 15:59 UTC (permalink / raw)
To: u-boot
As no gpio.h is defined for this architecture, to avoid
compilation failure, do not include <asm/arch/gpio.h> for
arch bcmns3.
Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
arch/arm/include/asm/gpio.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/gpio.h b/arch/arm/include/asm/gpio.h
index 333e407b66..c067eeb649 100644
--- a/arch/arm/include/asm/gpio.h
+++ b/arch/arm/include/asm/gpio.h
@@ -6,7 +6,7 @@
!defined(CONFIG_ARCH_LS1088A) && !defined(CONFIG_ARCH_ASPEED) && \
!defined(CONFIG_ARCH_LS1012A) && !defined(CONFIG_ARCH_LS1043A) && \
!defined(CONFIG_ARCH_LS1046A) && !defined(CONFIG_ARCH_U8500) && \
- !defined(CONFIG_CORTINA_PLATFORM)
+ !defined(CONFIG_CORTINA_PLATFORM) && !defined(CONFIG_TARGET_BCMNS3)
#include <asm/arch/gpio.h>
#endif
#include <asm-generic/gpio.h>
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v5 1/2] drivers: gpio: add broadcom iproc gpio driver support
2020-05-04 15:59 ` [PATCH v5 1/2] drivers: gpio: add broadcom iproc gpio driver support Rayagonda Kokatanur
@ 2020-05-04 19:26 ` Simon Glass
2020-05-05 6:32 ` Rayagonda Kokatanur
2020-05-05 16:02 ` Simon Glass
1 sibling, 1 reply; 8+ messages in thread
From: Simon Glass @ 2020-05-04 19:26 UTC (permalink / raw)
To: u-boot
Hi Rayagonda,
On Mon, 4 May 2020 at 10:00, Rayagonda Kokatanur
<rayagonda.kokatanur@broadcom.com> wrote:
>
> Add gpio driver support for Broadcom iproc-based socs.
>
This seems to be missing a change log. Can I suggest using patman to
make this automatic?
> 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 | 275 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 287 insertions(+)
> create mode 100644 drivers/gpio/iproc_gpio.c
>
Regards,
Simon
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v5 1/2] drivers: gpio: add broadcom iproc gpio driver support
2020-05-04 19:26 ` Simon Glass
@ 2020-05-05 6:32 ` Rayagonda Kokatanur
2020-05-05 16:02 ` Simon Glass
0 siblings, 1 reply; 8+ messages in thread
From: Rayagonda Kokatanur @ 2020-05-05 6:32 UTC (permalink / raw)
To: u-boot
Hi Simon,
On Tue, May 5, 2020 at 12:57 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Rayagonda,
>
> On Mon, 4 May 2020 at 10:00, Rayagonda Kokatanur
> <rayagonda.kokatanur@broadcom.com> wrote:
> >
> > Add gpio driver support for Broadcom iproc-based socs.
> >
>
> This seems to be missing a change log. Can I suggest using patman to
> make this automatic?
All change logs are present in the cover letter.
Please let me know if I still need to have a change log for individual patch.
>
> > 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 | 275 ++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 287 insertions(+)
> > create mode 100644 drivers/gpio/iproc_gpio.c
> >
> Regards,
> Simon
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v5 1/2] drivers: gpio: add broadcom iproc gpio driver support
2020-05-04 15:59 ` [PATCH v5 1/2] drivers: gpio: add broadcom iproc gpio driver support Rayagonda Kokatanur
2020-05-04 19:26 ` Simon Glass
@ 2020-05-05 16:02 ` Simon Glass
2020-05-05 17:44 ` Rayagonda Kokatanur
1 sibling, 1 reply; 8+ messages in thread
From: Simon Glass @ 2020-05-05 16:02 UTC (permalink / raw)
To: u-boot
Hi Rayagonda,
On Mon, 4 May 2020 at 10:00, 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 | 275 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 287 insertions(+)
> create mode 100644 drivers/gpio/iproc_gpio.c
>
Reviewed-by: Simon Glass <sjg@chromium.org>
nits and suggestions below
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 2081520f42..57d2cd2e75 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -135,6 +135,17 @@ config IMX_RGPIO2P
> help
> This driver supports i.MX7ULP Rapid GPIO2P controller.
>
> +config IPROC_GPIO
> + bool "Broadcom iProc GPIO driver(without pinconf)"
> + default n
> + help
> + The Broadcom iProc based SoCs- Cygnus, NS2, NS3, NSP and Stingray,
> + use the same GPIO Controller IP hence this driver could be used
> + for all.
> +
> + The Broadcom iProc based SoCs have multiple GPIO controllers and only
> + the always-ON GPIO controller (CRMU/AON) is supported by this driver.
> +
> config HSDK_CREG_GPIO
> bool "HSDK CREG GPIO griver"
> depends on DM_GPIO
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 7638259007..5dc5849477 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_CORTINA_GPIO) += cortina_gpio.o
> obj-$(CONFIG_INTEL_GPIO) += intel_gpio.o
> obj-$(CONFIG_INTEL_ICH6_GPIO) += intel_ich6_gpio.o
> obj-$(CONFIG_INTEL_BROADWELL_GPIO) += intel_broadwell_gpio.o
> +obj-$(CONFIG_IPROC_GPIO) += iproc_gpio.o
> obj-$(CONFIG_KIRKWOOD_GPIO) += kw_gpio.o
> obj-$(CONFIG_KONA_GPIO) += kona_gpio.o
> obj-$(CONFIG_MARVELL_GPIO) += mvgpio.o
> diff --git a/drivers/gpio/iproc_gpio.c b/drivers/gpio/iproc_gpio.c
> new file mode 100644
> index 0000000000..f75831d6e7
> --- /dev/null
> +++ b/drivers/gpio/iproc_gpio.c
> @@ -0,0 +1,275 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2020 Broadcom
> + */
> +
> +#include <common.h>
> +#include <errno.h>
> +#include <asm/gpio.h>
> +#include <asm/io.h>
> +#include <dm.h>
> +#include <dm/device_compat.h>
> +#include <dm/devres.h>
> +#include <dm/pinctrl.h>
Do you need that one?
> +
> +/*
> + * There are five GPIO bank register. Each bank can configure max of 32 gpios.
> + * BANK0 - gpios 0 to 31
> + * BANK1 - gpios 32 to 63
> + * BANK2 - gpios 64 to 95
> + * BANK3 - gpios 96 to 127
> + * BANK4 - gpios 128 to 150
> + *
> + * Offset difference between consecutive bank register is 0x200
> + */
> +#define NGPIO_PER_BANK 32
> +#define GPIO_BANK_SIZE 0x200
> +#define GPIO_BANK(pin) ((pin) / NGPIO_PER_BANK)
> +#define IPROC_GPIO_SHIFT(pin) ((pin) % NGPIO_PER_BANK)
> +#define IPROC_GPIO_REG(pin, reg) (GPIO_BANK_SIZE * GPIO_BANK(pin) + (reg))
> +
> +/* device register offset */
> +#define IPROC_GPIO_DATA_IN_OFFSET 0x00
> +#define IPROC_GPIO_DATA_OUT_OFFSET 0x04
> +#define IPROC_GPIO_OUT_EN_OFFSET 0x08
> +
> +/**
> + * struct iproc_gpio_pctrl_map - mapping between gpio and pinctrl specfied
> + * using gpio-ranges parameter in dt.
> + * @gpio_pin: start of gpio number in gpio-ranges
> + * @pctrl_pin: start of pinctrl number in gpio-ranges
> + * @npins: total number of pins in gpio-ranges
> + * @node: list node
> + */
> +struct iproc_gpio_pctrl_map {
> + u32 gpio_pin;
> + u32 pctrl_pin;
> + u32 npins;
> + struct list_head node;
> +};
> +
> +/**
> + * struct iproc_gpio_pctrl_map - gpio device instance
> + * @pinctrl_dev:pointer to pinctrl device
> + * @gpiomap: list node having mapping between gpio and pinctrl
> + * @base: I/O register base address of gpio device
> + * @name: gpio device name, ex GPIO0, GPIO1
> + * @ngpios: total number of gpios
> + */
> +struct iproc_gpio_platdata {
> + struct udevice *pinctrl_dev;
> + struct list_head gpiomap;
> + void __iomem *base;
> + char *name;
> + u32 ngpios;
> +};
> +
> +/**
> + * iproc_gpio_set_bit - set or clear one bit (corresponding to the GPIO pin)
Please fit the title on a single line. You can add more detail later. E.g.
iproc_gpio_set_bit() - set or clear one bit in an iproc GPIO register
The bit relates to a GPIO pin
> + * in a iproc GPIO register
> + *
> + * @iproc_gpio: Iproc GPIO device
But here you are actually passing plat
> + * @reg: register offset
> + * @gpio: GPIO pin
> + * @set: set or clear
> + */
> +static inline void iproc_gpio_set_bit(struct iproc_gpio_platdata *plat,
> + u32 reg, u32 gpio, bool set)
> +{
> + u32 offset = IPROC_GPIO_REG(gpio, reg);
> + u32 shift = IPROC_GPIO_SHIFT(gpio);
> +
> + clrsetbits_le32(plat->base + offset, BIT(shift),
> + (set ? BIT(shift) : 0));
> +}
> +
> +static inline bool iproc_gpio_get_bit(struct iproc_gpio_platdata *plat,
> + u32 reg, u32 gpio)
> +{
> + u32 offset = IPROC_GPIO_REG(gpio, reg);
> + u32 shift = IPROC_GPIO_SHIFT(gpio);
> +
> + return readl(plat->base + offset) & BIT(shift);
> +}
> +
> +static u32 iproc_get_pctrl_from_gpio(struct iproc_gpio_platdata *plat, u32 gpio)
function comment
> +{
> + struct iproc_gpio_pctrl_map *range = NULL;
> + struct list_head *pos, *tmp;
> + u32 ret = 0;
> +
> + list_for_each_safe(pos, tmp, &plat->gpiomap) {
> + range = list_entry(pos, struct iproc_gpio_pctrl_map, node);
> + if (gpio == range->gpio_pin ||
> + gpio < (range->gpio_pin + range->npins)) {
> + ret = range->pctrl_pin + (gpio - range->gpio_pin);
> + break;
> + }
> + }
> +
> + return ret;
> +}
> +
> +static int iproc_get_gpio_pctrl_mapping(struct udevice *dev)
function comment
> +{
> + struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
> + struct iproc_gpio_pctrl_map *range = NULL;
> + struct ofnode_phandle_args args;
> + int index = 0, ret;
> +
> + for (;; index++) {
> + ret = dev_read_phandle_with_args(dev, "gpio-ranges",
> + NULL, 3, index, &args);
> + if (ret)
> + break;
> +
> + range = devm_kzalloc(dev, sizeof(*range), GFP_KERNEL);
> + if (!range)
> + return -ENOMEM;
> +
> + range->gpio_pin = args.args[0];
> + range->pctrl_pin = args.args[1];
> + range->npins = args.args[2];
> + list_add_tail(&range->node, &plat->gpiomap);
> + }
> +
> + return 0;
> +}
> +
> +static int iproc_gpio_request(struct udevice *dev, u32 gpio, const char *label)
> +{
> + struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
> + u32 pctrl;
> +
> + if (!plat->pinctrl_dev)
comment why you are doing this return. I think it is because it is a
top-level device?
> + return 0;
> +
> + pctrl = iproc_get_pctrl_from_gpio(plat, gpio);
> +
> + return pinctrl_request(plat->pinctrl_dev, pctrl, 0);
> +}
> +
> +static int iproc_gpio_direction_input(struct udevice *dev, u32 gpio)
> +{
> + struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
> +
> + iproc_gpio_set_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio, false);
You might consider dropping the IPROC_GPIO prefix on these, or at
least the IPROC prefix. They are close to this file so there is no
need for all the boilerplate.
> + dev_dbg(dev, "gpio:%u set input\n", gpio);
> +
> + return 0;
> +}
> +
> +static int iproc_gpio_direction_output(struct udevice *dev, u32 gpio, int value)
> +{
> + struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
> +
> + iproc_gpio_set_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio, true);
> + iproc_gpio_set_bit(plat, IPROC_GPIO_DATA_OUT_OFFSET, gpio, value);
> + dev_dbg(dev, "gpio:%u set output, value:%d\n", gpio, value);
> +
> + return 0;
> +}
> +
> +static int iproc_gpio_get_value(struct udevice *dev, u32 gpio)
> +{
> + struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
> + int value;
> +
> + value = iproc_gpio_get_bit(plat, IPROC_GPIO_DATA_IN_OFFSET, gpio);
> + dev_dbg(dev, "gpio:%u get, value:%d\n", gpio, value);
> +
> + return value;
> +}
> +
> +static int iproc_gpio_set_value(struct udevice *dev, u32 gpio, int value)
> +{
> + struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
> +
> + if (iproc_gpio_get_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio))
> + iproc_gpio_set_bit(plat, IPROC_GPIO_DATA_OUT_OFFSET, gpio,
> + value);
> +
> + dev_dbg(dev, "gpio:%u set, value:%d\n", gpio, value);
> + return 0;
> +}
> +
> +static int iproc_gpio_get_function(struct udevice *dev, u32 gpio)
> +{
> + struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
> +
> + if (iproc_gpio_get_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio))
> + return GPIOF_OUTPUT;
> + else
> + return GPIOF_INPUT;
> +}
> +
> +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__);
Note that this adds size to the driver. If you don't care about that
it is fine, otherwise consider using log_debug()
> + return ret;
> + }
> +
> + uclass_get_device_by_phandle(UCLASS_PINCTRL, dev, "gpio-ranges",
> + &plat->pinctrl_dev);
> + if (ret < 0) {
> + dev_err(dev, "%s: Failed to get pinctrl phandle\n", __func__);
> + return ret;
> + }
> +
> + INIT_LIST_HEAD(&plat->gpiomap);
> + ret = iproc_get_gpio_pctrl_mapping(dev);
> + if (ret < 0) {
> + 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);
> + if (!plat->name)
> + 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;
> +}
> +
> +static const struct dm_gpio_ops iproc_gpio_ops = {
> + .request = iproc_gpio_request,
> + .direction_input = iproc_gpio_direction_input,
> + .direction_output = iproc_gpio_direction_output,
> + .get_value = iproc_gpio_get_value,
> + .set_value = iproc_gpio_set_value,
> + .get_function = iproc_gpio_get_function,
> +};
> +
> +static const struct udevice_id iproc_gpio_ids[] = {
> + { .compatible = "brcm,iproc-gpio" },
> + { }
> +};
> +
> +U_BOOT_DRIVER(iproc_gpio) = {
> + .name = "iproc_gpio",
> + .id = UCLASS_GPIO,
> + .of_match = iproc_gpio_ids,
> + .ops = &iproc_gpio_ops,
> + .ofdata_to_platdata = iproc_gpio_ofdata_to_platdata,
> + .platdata_auto_alloc_size = sizeof(struct iproc_gpio_platdata),
> +};
> --
> 2.17.1
>
Regards,
Simon
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v5 1/2] drivers: gpio: add broadcom iproc gpio driver support
2020-05-05 6:32 ` Rayagonda Kokatanur
@ 2020-05-05 16:02 ` Simon Glass
0 siblings, 0 replies; 8+ messages in thread
From: Simon Glass @ 2020-05-05 16:02 UTC (permalink / raw)
To: u-boot
Hi Rayagonda,
On Tue, 5 May 2020 at 00:32, Rayagonda Kokatanur
<rayagonda.kokatanur@broadcom.com> wrote:
>
> Hi Simon,
>
> On Tue, May 5, 2020 at 12:57 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Rayagonda,
> >
> > On Mon, 4 May 2020 at 10:00, Rayagonda Kokatanur
> > <rayagonda.kokatanur@broadcom.com> wrote:
> > >
> > > Add gpio driver support for Broadcom iproc-based socs.
> > >
> >
> > This seems to be missing a change log. Can I suggest using patman to
> > make this automatic?
>
> All change logs are present in the cover letter.
> Please let me know if I still need to have a change log for individual patch.
Yes please. As I mentioned, patman does this for you.
- Simon
>
> >
> > > 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 | 275 ++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 287 insertions(+)
> > > create mode 100644 drivers/gpio/iproc_gpio.c
> > >
> > Regards,
> > Simon
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v5 1/2] drivers: gpio: add broadcom iproc gpio driver support
2020-05-05 16:02 ` Simon Glass
@ 2020-05-05 17:44 ` Rayagonda Kokatanur
0 siblings, 0 replies; 8+ messages in thread
From: Rayagonda Kokatanur @ 2020-05-05 17:44 UTC (permalink / raw)
To: u-boot
Hi Simon,
On Tue, May 5, 2020 at 9:32 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Rayagonda,
>
> On Mon, 4 May 2020 at 10:00, 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 | 275 ++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 287 insertions(+)
> > create mode 100644 drivers/gpio/iproc_gpio.c
> >
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> nits and suggestions below
>
> > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> > index 2081520f42..57d2cd2e75 100644
> > --- a/drivers/gpio/Kconfig
> > +++ b/drivers/gpio/Kconfig
> > @@ -135,6 +135,17 @@ config IMX_RGPIO2P
> > help
> > This driver supports i.MX7ULP Rapid GPIO2P controller.
> >
> > +config IPROC_GPIO
> > + bool "Broadcom iProc GPIO driver(without pinconf)"
> > + default n
> > + help
> > + The Broadcom iProc based SoCs- Cygnus, NS2, NS3, NSP and Stingray,
> > + use the same GPIO Controller IP hence this driver could be used
> > + for all.
> > +
> > + The Broadcom iProc based SoCs have multiple GPIO controllers and only
> > + the always-ON GPIO controller (CRMU/AON) is supported by this driver.
> > +
> > config HSDK_CREG_GPIO
> > bool "HSDK CREG GPIO griver"
> > depends on DM_GPIO
> > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> > index 7638259007..5dc5849477 100644
> > --- a/drivers/gpio/Makefile
> > +++ b/drivers/gpio/Makefile
> > @@ -19,6 +19,7 @@ obj-$(CONFIG_CORTINA_GPIO) += cortina_gpio.o
> > obj-$(CONFIG_INTEL_GPIO) += intel_gpio.o
> > obj-$(CONFIG_INTEL_ICH6_GPIO) += intel_ich6_gpio.o
> > obj-$(CONFIG_INTEL_BROADWELL_GPIO) += intel_broadwell_gpio.o
> > +obj-$(CONFIG_IPROC_GPIO) += iproc_gpio.o
> > obj-$(CONFIG_KIRKWOOD_GPIO) += kw_gpio.o
> > obj-$(CONFIG_KONA_GPIO) += kona_gpio.o
> > obj-$(CONFIG_MARVELL_GPIO) += mvgpio.o
> > diff --git a/drivers/gpio/iproc_gpio.c b/drivers/gpio/iproc_gpio.c
> > new file mode 100644
> > index 0000000000..f75831d6e7
> > --- /dev/null
> > +++ b/drivers/gpio/iproc_gpio.c
> > @@ -0,0 +1,275 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2020 Broadcom
> > + */
> > +
> > +#include <common.h>
> > +#include <errno.h>
> > +#include <asm/gpio.h>
> > +#include <asm/io.h>
> > +#include <dm.h>
> > +#include <dm/device_compat.h>
> > +#include <dm/devres.h>
> > +#include <dm/pinctrl.h>
>
> Do you need that one?
Yes #include <dm/pinctrl.h> is required for pinctrl_request() used in
iproc_gpio_request() function.
>
> > +
> > +/*
> > + * There are five GPIO bank register. Each bank can configure max of 32 gpios.
> > + * BANK0 - gpios 0 to 31
> > + * BANK1 - gpios 32 to 63
> > + * BANK2 - gpios 64 to 95
> > + * BANK3 - gpios 96 to 127
> > + * BANK4 - gpios 128 to 150
> > + *
> > + * Offset difference between consecutive bank register is 0x200
> > + */
> > +#define NGPIO_PER_BANK 32
> > +#define GPIO_BANK_SIZE 0x200
> > +#define GPIO_BANK(pin) ((pin) / NGPIO_PER_BANK)
> > +#define IPROC_GPIO_SHIFT(pin) ((pin) % NGPIO_PER_BANK)
> > +#define IPROC_GPIO_REG(pin, reg) (GPIO_BANK_SIZE * GPIO_BANK(pin) + (reg))
> > +
> > +/* device register offset */
> > +#define IPROC_GPIO_DATA_IN_OFFSET 0x00
> > +#define IPROC_GPIO_DATA_OUT_OFFSET 0x04
> > +#define IPROC_GPIO_OUT_EN_OFFSET 0x08
> > +
> > +/**
> > + * struct iproc_gpio_pctrl_map - mapping between gpio and pinctrl specfied
> > + * using gpio-ranges parameter in dt.
> > + * @gpio_pin: start of gpio number in gpio-ranges
> > + * @pctrl_pin: start of pinctrl number in gpio-ranges
> > + * @npins: total number of pins in gpio-ranges
> > + * @node: list node
> > + */
> > +struct iproc_gpio_pctrl_map {
> > + u32 gpio_pin;
> > + u32 pctrl_pin;
> > + u32 npins;
> > + struct list_head node;
> > +};
> > +
> > +/**
> > + * struct iproc_gpio_pctrl_map - gpio device instance
> > + * @pinctrl_dev:pointer to pinctrl device
> > + * @gpiomap: list node having mapping between gpio and pinctrl
> > + * @base: I/O register base address of gpio device
> > + * @name: gpio device name, ex GPIO0, GPIO1
> > + * @ngpios: total number of gpios
> > + */
> > +struct iproc_gpio_platdata {
> > + struct udevice *pinctrl_dev;
> > + struct list_head gpiomap;
> > + void __iomem *base;
> > + char *name;
> > + u32 ngpios;
> > +};
> > +
> > +/**
> > + * iproc_gpio_set_bit - set or clear one bit (corresponding to the GPIO pin)
>
> Please fit the title on a single line. You can add more detail later. E.g.
>
> iproc_gpio_set_bit() - set or clear one bit in an iproc GPIO register
>
> The bit relates to a GPIO pin
>
>
>
> > + * in a iproc GPIO register
> > + *
> > + * @iproc_gpio: Iproc GPIO device
>
> But here you are actually passing plat
>
> > + * @reg: register offset
> > + * @gpio: GPIO pin
> > + * @set: set or clear
> > + */
> > +static inline void iproc_gpio_set_bit(struct iproc_gpio_platdata *plat,
> > + u32 reg, u32 gpio, bool set)
> > +{
> > + u32 offset = IPROC_GPIO_REG(gpio, reg);
> > + u32 shift = IPROC_GPIO_SHIFT(gpio);
> > +
> > + clrsetbits_le32(plat->base + offset, BIT(shift),
> > + (set ? BIT(shift) : 0));
> > +}
> > +
> > +static inline bool iproc_gpio_get_bit(struct iproc_gpio_platdata *plat,
> > + u32 reg, u32 gpio)
> > +{
> > + u32 offset = IPROC_GPIO_REG(gpio, reg);
> > + u32 shift = IPROC_GPIO_SHIFT(gpio);
> > +
> > + return readl(plat->base + offset) & BIT(shift);
> > +}
> > +
> > +static u32 iproc_get_pctrl_from_gpio(struct iproc_gpio_platdata *plat, u32 gpio)
>
> function comment
>
> > +{
> > + struct iproc_gpio_pctrl_map *range = NULL;
> > + struct list_head *pos, *tmp;
> > + u32 ret = 0;
> > +
> > + list_for_each_safe(pos, tmp, &plat->gpiomap) {
> > + range = list_entry(pos, struct iproc_gpio_pctrl_map, node);
> > + if (gpio == range->gpio_pin ||
> > + gpio < (range->gpio_pin + range->npins)) {
> > + ret = range->pctrl_pin + (gpio - range->gpio_pin);
> > + break;
> > + }
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int iproc_get_gpio_pctrl_mapping(struct udevice *dev)
>
> function comment
>
> > +{
> > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
> > + struct iproc_gpio_pctrl_map *range = NULL;
> > + struct ofnode_phandle_args args;
> > + int index = 0, ret;
> > +
> > + for (;; index++) {
> > + ret = dev_read_phandle_with_args(dev, "gpio-ranges",
> > + NULL, 3, index, &args);
> > + if (ret)
> > + break;
> > +
> > + range = devm_kzalloc(dev, sizeof(*range), GFP_KERNEL);
> > + if (!range)
> > + return -ENOMEM;
> > +
> > + range->gpio_pin = args.args[0];
> > + range->pctrl_pin = args.args[1];
> > + range->npins = args.args[2];
> > + list_add_tail(&range->node, &plat->gpiomap);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int iproc_gpio_request(struct udevice *dev, u32 gpio, const char *label)
> > +{
> > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
> > + u32 pctrl;
> > +
> > + if (!plat->pinctrl_dev)
>
> comment why you are doing this return. I think it is because it is a
> top-level device?
>
> > + return 0;
> > +
> > + pctrl = iproc_get_pctrl_from_gpio(plat, gpio);
> > +
> > + return pinctrl_request(plat->pinctrl_dev, pctrl, 0);
> > +}
> > +
> > +static int iproc_gpio_direction_input(struct udevice *dev, u32 gpio)
> > +{
> > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
> > +
> > + iproc_gpio_set_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio, false);
>
> You might consider dropping the IPROC_GPIO prefix on these, or at
> least the IPROC prefix. They are close to this file so there is no
> need for all the boilerplate.
>
> > + dev_dbg(dev, "gpio:%u set input\n", gpio);
> > +
> > + return 0;
> > +}
> > +
> > +static int iproc_gpio_direction_output(struct udevice *dev, u32 gpio, int value)
> > +{
> > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
> > +
> > + iproc_gpio_set_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio, true);
> > + iproc_gpio_set_bit(plat, IPROC_GPIO_DATA_OUT_OFFSET, gpio, value);
> > + dev_dbg(dev, "gpio:%u set output, value:%d\n", gpio, value);
> > +
> > + return 0;
> > +}
> > +
> > +static int iproc_gpio_get_value(struct udevice *dev, u32 gpio)
> > +{
> > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
> > + int value;
> > +
> > + value = iproc_gpio_get_bit(plat, IPROC_GPIO_DATA_IN_OFFSET, gpio);
> > + dev_dbg(dev, "gpio:%u get, value:%d\n", gpio, value);
> > +
> > + return value;
> > +}
> > +
> > +static int iproc_gpio_set_value(struct udevice *dev, u32 gpio, int value)
> > +{
> > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
> > +
> > + if (iproc_gpio_get_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio))
> > + iproc_gpio_set_bit(plat, IPROC_GPIO_DATA_OUT_OFFSET, gpio,
> > + value);
> > +
> > + dev_dbg(dev, "gpio:%u set, value:%d\n", gpio, value);
> > + return 0;
> > +}
> > +
> > +static int iproc_gpio_get_function(struct udevice *dev, u32 gpio)
> > +{
> > + struct iproc_gpio_platdata *plat = dev_get_platdata(dev);
> > +
> > + if (iproc_gpio_get_bit(plat, IPROC_GPIO_OUT_EN_OFFSET, gpio))
> > + return GPIOF_OUTPUT;
> > + else
> > + return GPIOF_INPUT;
> > +}
> > +
> > +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__);
>
> Note that this adds size to the driver. If you don't care about that
> it is fine, otherwise consider using log_debug()
Thanks for the info.
Since it is an important error message and I feel it should be dev_err.
Rest of the review comments will fix and send v6.
Regards,
Rayagonda
>
> > + return ret;
> > + }
> > +
> > + uclass_get_device_by_phandle(UCLASS_PINCTRL, dev, "gpio-ranges",
> > + &plat->pinctrl_dev);
> > + if (ret < 0) {
> > + dev_err(dev, "%s: Failed to get pinctrl phandle\n", __func__);
> > + return ret;
> > + }
> > +
> > + INIT_LIST_HEAD(&plat->gpiomap);
> > + ret = iproc_get_gpio_pctrl_mapping(dev);
> > + if (ret < 0) {
> > + 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);
> > + if (!plat->name)
> > + 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;
> > +}
> > +
> > +static const struct dm_gpio_ops iproc_gpio_ops = {
> > + .request = iproc_gpio_request,
> > + .direction_input = iproc_gpio_direction_input,
> > + .direction_output = iproc_gpio_direction_output,
> > + .get_value = iproc_gpio_get_value,
> > + .set_value = iproc_gpio_set_value,
> > + .get_function = iproc_gpio_get_function,
> > +};
> > +
> > +static const struct udevice_id iproc_gpio_ids[] = {
> > + { .compatible = "brcm,iproc-gpio" },
> > + { }
> > +};
> > +
> > +U_BOOT_DRIVER(iproc_gpio) = {
> > + .name = "iproc_gpio",
> > + .id = UCLASS_GPIO,
> > + .of_match = iproc_gpio_ids,
> > + .ops = &iproc_gpio_ops,
> > + .ofdata_to_platdata = iproc_gpio_ofdata_to_platdata,
> > + .platdata_auto_alloc_size = sizeof(struct iproc_gpio_platdata),
> > +};
> > --
> > 2.17.1
> >
>
> Regards,
> Simon
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-05-05 17:44 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-04 15:59 [PATCH v5 0/2] add brcm iproc gpio driver Rayagonda Kokatanur
2020-05-04 15:59 ` [PATCH v5 1/2] drivers: gpio: add broadcom iproc gpio driver support Rayagonda Kokatanur
2020-05-04 19:26 ` Simon Glass
2020-05-05 6:32 ` Rayagonda Kokatanur
2020-05-05 16:02 ` Simon Glass
2020-05-05 16:02 ` Simon Glass
2020-05-05 17:44 ` Rayagonda Kokatanur
2020-05-04 15:59 ` [PATCH v5 2/2] gpio: do not include <asm/arch/gpio.h> on TARGET_BCMNS3 Rayagonda Kokatanur
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.